[ENH]: Execute task with no backfill or incremental #5867

tanujnay112 · 2025-11-14T23:21:54Z

Description of changes

Summarize the changes made by this PR.

This change introduces an AttachedFunctionOrchestrator that does the following chain of operators

GetAttachedFunction -> GetCollectionAndSegments for output collection -> ExecuteAttachedFunction -> MaterializeLogs

It edits the RegisterOrchestrator to spawn a FinishAttachedTask operators that flushes compaction for the input + output collections and flushes updated function data.

The compact method in compact.rs has been edited to launch an AttachedFunctionOrchestrator in parallel on the results of the initial LogFetchOrchestrator. This orchestrator returns a chunk of MaterializedLog records that get applied via another instance of ApplyLogOrchestrator.

The above runs in parallel to the normal compaction workflow which simply runs an ApplyLogsOrchestrator on the results of the initial LogFetchOrchestrator.

The above two threads return a CollectionRegisterInfo. The function-related thread also returns a FunctionContext. The two threads are joined and each of these structures are passed onto the RegisterOrchestrator for completion.

CompactionContext.collection_info has been replaced by CompactionContext.input_collection_info, CompactionContext.output_collection_info

to reflect the fact that "compactions" can be pulling data from one collection and compacting to another. ApplyLogsOrchestrator always applies given logs to the collection specified by CompactionContext.output_collection_info

Hence, we take care to set this field to the appropriate collection before calling run_apply_logs in each thread.

Improvements & Bug fixes
- ...
New functionality
- ...

Test plan

How are these changes tested?

Tests pass locally with pytest for python, yarn test for js, cargo test for rust

Migration plan

Are there any migrations, or any forwards/backwards compatibility changes needed in order to make sure this change deploys reliably?

Observability plan

What is the plan to instrument and monitor this change?

Documentation Changes

Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the _docs section?_

github-actions · 2025-11-14T23:22:09Z

tanujnay112 · 2025-11-14T23:22:16Z

[ENH]: Make all functions incremental #5893
[ENH]: Execute task with no backfill or incremental #5867 👈 (View in Graphite)
[ENH]: Modified AttachFunction to do 2PC on a new is_ready column #5872 : 1 other dependent PR (#5884 )
[CHORE]: Remove next_run from attached_functions #5871
[CHORE]: Disable S3heap service and remove nonce-related logic #5866
[ENH]: Refactor compactor into three chained orchestrators #5831
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

rust/worker/src/execution/operators/register.rs

rust/worker/src/execution/orchestration/compact.rs

rust/worker/src/execution/operators/execute_task.rs

HammadB

Not hitting approve as I don't have bandwidth to review the size of diff and others have read it

HammadB · 2025-11-21T19:47:20Z

go/pkg/sysdb/coordinator/task.go


-// AttachFunction creates a new attached function in the database
+// AttachFunction creates an output collection and attached function in a single transaction
 func (s *Coordinator) AttachFunction(ctx context.Context, req *coordinatorpb.AttachFunctionRequest) (*coordinatorpb.AttachFunctionResponse, error) {


Do we handle soft deletes for

Attaching

Comapction flush?

Yes, compaction flushing uses the same logic as before which fails the flush if is_deleted is true. Similarly, a flush on a function that doesn't exist or is soft deleted will abort the transaction.

Attaching a function does not interact with soft deleted functions and soft deleted functions are renamed with a "deleted" prefix just how collections do.

go/pkg/sysdb/grpc/collection_service.go

HammadB · 2025-11-21T19:49:58Z

go/pkg/sysdb/grpc/collection_service.go

+			for key, filePath := range flushSegmentCompaction.FilePaths {
+				filePaths[key] = filePath.Paths
+			}
+			segmentCompactionInfo = append(segmentCompactionInfo, &model.FlushSegmentCompaction{


Do we properly handle empty compactions and the like here?

yes, it's the same as normal compactions. That case is tested by compact.rs::test_compaction_with_empty_logs_from_inserts_and_deletes

rust/sysdb/src/sysdb.rs

rust/sysdb/src/test_sysdb.rs

rust/types/src/flush.rs

HammadB · 2025-11-21T20:02:26Z

rust/worker/src/execution/operators/execute_task.rs

        };

-        Ok(Chunk::new(Arc::new([log_record])))
+        Ok(Chunk::new(std::sync::Arc::from(vec![output_record])))


Also please just import Arc

rust/worker/src/execution/operators/execute_task.rs

rust/worker/src/execution/orchestration/attached_function_orchestrator.rs

propel-code-bot · 2025-11-21T22:24:56Z

rust/worker/src/execution/orchestration/register_orchestrator.rs

+        collection_register_infos: Vec<CollectionRegisterInfo>,
+        function_context: Option<FunctionContext>,
    ) -> Self {


[BestPractice]

Missing validation for collection_register_infos bounds: The function validates length is not 0 or >2, but then immediately calls .first() without handling the empty case that could theoretically reach line 207 if the validation is bypassed. While the validation should prevent this, defensive programming suggests:

let output_collection_register_info = self.collection_register_infos.first() .ok_or_else(|| RegisterOrchestratorError::InvariantViolation( "No collection register info found" ))?;

This is already handled correctly. However, the validation message at line 138 says "Invalid number of collection register infos" which is vague. Be more specific: "Expected 1 or 2 collection register infos, got {count}"

Context for Agents

**Missing validation for collection_register_infos bounds**: The function validates length is not 0 or >2, but then immediately calls `.first()` without handling the empty case that could theoretically reach line 207 if the validation is bypassed. While the validation should prevent this, defensive programming suggests: ```rust let output_collection_register_info = self.collection_register_infos.first() .ok_or_else(|| RegisterOrchestratorError::InvariantViolation( "No collection register info found" ))?; ``` This is already handled correctly. However, the validation message at line 138 says "Invalid number of collection register infos" which is vague. Be more specific: "Expected 1 or 2 collection register infos, got {count}" File: rust/worker/src/execution/orchestration/register_orchestrator.rs Line: 138

rust/worker/src/execution/functions/statistics.rs

Sicheng-Pan

Lgtm

propel-code-bot · 2025-11-21T23:57:24Z

go/pkg/sysdb/coordinator/task.go

 		return nil, status.Errorf(codes.Internal, "attached function has invalid completion_offset: %d", attachedFunction.CompletionOffset)
 	}

+	if !attachedFunction.IsReady {


[BestPractice]

The check if !attachedFunction.IsReady inside attachedFunctionToProto appears to be redundant. The callers of this function (GetAttachedFunctionByName, ListAttachedFunctions, GetAttachedFunctionByUuid) use data access methods (GetByName, GetByCollectionID, GetByID) that are already specified to return only "ready" attached functions.

This redundant check returns a generic codes.Internal error, which could be confusing for clients. If an unready function is passed here, it signifies an internal logic error. Consider removing this check to simplify the code and rely on the data access layer to enforce the "ready" contract. If the check is kept for defensiveness, a panic might be more appropriate to signal the internal invariant violation.

Context for Agents

The check `if !attachedFunction.IsReady` inside `attachedFunctionToProto` appears to be redundant. The callers of this function (`GetAttachedFunctionByName`, `ListAttachedFunctions`, `GetAttachedFunctionByUuid`) use data access methods (`GetByName`, `GetByCollectionID`, `GetByID`) that are already specified to return only "ready" attached functions. This redundant check returns a generic `codes.Internal` error, which could be confusing for clients. If an unready function is passed here, it signifies an internal logic error. Consider removing this check to simplify the code and rely on the data access layer to enforce the "ready" contract. If the check is kept for defensiveness, a panic might be more appropriate to signal the internal invariant violation. File: go/pkg/sysdb/coordinator/task.go Line: 225

propel-code-bot · 2025-11-21T23:57:24Z

rust/worker/src/execution/orchestration/register_orchestrator.rs

+        if let Some(function_context) = &self.function_context {
+            vec![(
+                wrap(
+                    FinishAttachedFunctionOperator::new(),


[BestPractice]

There's a minor naming inconsistency. The PR description refers to a FinishAttachedTask operator, but the implementation is named FinishAttachedFunctionOperator. For consistency with the documentation and intent, consider renaming FinishAttachedFunctionOperator to FinishAttachedTaskOperator.

Context for Agents

There's a minor naming inconsistency. The PR description refers to a `FinishAttachedTask` operator, but the implementation is named `FinishAttachedFunctionOperator`. For consistency with the documentation and intent, consider renaming `FinishAttachedFunctionOperator` to `FinishAttachedTaskOperator`. File: rust/worker/src/execution/orchestration/register_orchestrator.rs Line: 184

rust/worker/src/execution/orchestration/compact.rs

tanujnay112 mentioned this pull request Nov 14, 2025

[ENH]: Refactor compactor into three chained orchestrators #5831

Merged

1 task

tanujnay112 mentioned this pull request Nov 14, 2025

[CHORE]: Disable S3heap service and remove nonce-related logic #5866

Merged

1 task

This comment has been minimized.

Sign in to view

tanujnay112 commented Nov 14, 2025

View reviewed changes

rust/worker/src/execution/operators/register.rs Outdated Show resolved Hide resolved

tanujnay112 commented Nov 15, 2025

View reviewed changes

rust/worker/src/execution/orchestration/compact.rs Show resolved Hide resolved

tanujnay112 changed the base branch from refactor_compactor to graphite-base/5867 November 16, 2025 23:26

tanujnay112 force-pushed the make_functions_great_again branch from 23911b3 to 41adcf1 Compare November 16, 2025 23:26

tanujnay112 changed the base branch from graphite-base/5867 to remove_next_run November 16, 2025 23:26

tanujnay112 mentioned this pull request Nov 16, 2025

[CHORE]: Remove next_run from attached_functions #5871

Merged

1 task

This comment has been minimized.

Sign in to view

tanujnay112 changed the base branch from remove_next_run to graphite-base/5867 November 17, 2025 00:47

tanujnay112 force-pushed the make_functions_great_again branch from 41adcf1 to e9fd0f2 Compare November 17, 2025 00:48

tanujnay112 changed the base branch from graphite-base/5867 to attach_fn2 November 17, 2025 00:48

tanujnay112 mentioned this pull request Nov 17, 2025

[ENH]: Modified AttachFunction to do 2PC on a new is_ready column #5872

Merged

1 task

tanujnay112 force-pushed the make_functions_great_again branch from e9fd0f2 to 64a563e Compare November 17, 2025 03:59

tanujnay112 force-pushed the attach_fn2 branch from 9bb916b to d0f93cf Compare November 17, 2025 03:59

tanujnay112 force-pushed the make_functions_great_again branch from 64a563e to 9bd6743 Compare November 17, 2025 04:28

tanujnay112 force-pushed the attach_fn2 branch 2 times, most recently from cf18054 to 5b066ac Compare November 17, 2025 04:31

tanujnay112 force-pushed the make_functions_great_again branch 2 times, most recently from 3ea4c7a to 56d4987 Compare November 17, 2025 05:01

tanujnay112 force-pushed the attach_fn2 branch 2 times, most recently from 105f707 to d9e7a48 Compare November 17, 2025 05:23

tanujnay112 force-pushed the make_functions_great_again branch from 56d4987 to 243f3f1 Compare November 17, 2025 05:23

This comment has been minimized.

Sign in to view

tanujnay112 force-pushed the make_functions_great_again branch 2 times, most recently from 8406711 to 205dbe0 Compare November 17, 2025 06:41

tanujnay112 force-pushed the attach_fn2 branch from d9e7a48 to 175e00e Compare November 17, 2025 06:41

tanujnay112 force-pushed the make_functions_great_again branch from 967ebb5 to 3646f99 Compare November 21, 2025 08:54

propel-code-bot bot reviewed Nov 21, 2025

View reviewed changes

rust/worker/src/execution/orchestration/compact.rs Show resolved Hide resolved

propel-code-bot bot reviewed Nov 21, 2025

View reviewed changes

rust/worker/src/execution/operators/execute_task.rs Show resolved Hide resolved

tanujnay112 mentioned this pull request Nov 21, 2025

[ENH]: Make all functions incremental #5893

Merged

1 task

tanujnay112 changed the base branch from attach_fn2 to graphite-base/5867 November 21, 2025 09:53

tanujnay112 force-pushed the graphite-base/5867 branch from b45774b to fa6f167 Compare November 21, 2025 09:53

tanujnay112 force-pushed the make_functions_great_again branch from 3646f99 to 09a23f0 Compare November 21, 2025 09:53

graphite-app bot changed the base branch from graphite-base/5867 to main November 21, 2025 09:54

tanujnay112 force-pushed the make_functions_great_again branch from 09a23f0 to 2bfcc0b Compare November 21, 2025 09:54

HammadB reviewed Nov 21, 2025

View reviewed changes

tanujnay112 force-pushed the make_functions_great_again branch from 2bfcc0b to cb021ee Compare November 21, 2025 22:19

propel-code-bot bot reviewed Nov 21, 2025

View reviewed changes

rust/worker/src/execution/orchestration/attached_function_orchestrator.rs Show resolved Hide resolved

propel-code-bot bot reviewed Nov 21, 2025

View reviewed changes

rust/worker/src/execution/functions/statistics.rs Show resolved Hide resolved

This comment has been minimized.

Sign in to view

tanujnay112 force-pushed the make_functions_great_again branch from cb021ee to af08c64 Compare November 21, 2025 22:31

This comment has been minimized.

Sign in to view

tanujnay112 force-pushed the make_functions_great_again branch from af08c64 to cb99156 Compare November 21, 2025 22:50

Sicheng-Pan approved these changes Nov 21, 2025

View reviewed changes

tanujnay112 force-pushed the make_functions_great_again branch from cb99156 to 9b3a17d Compare November 21, 2025 23:52

propel-code-bot bot reviewed Nov 21, 2025

View reviewed changes

tanujnay112 added 2 commits November 21, 2025 17:33

[ENH]: Execute task with no backfill or incremental

a0ea383

sicheng comments

da946db

tanujnay112 force-pushed the make_functions_great_again branch from 9b3a17d to e1c52e2 Compare November 22, 2025 01:34

address hammad + sicheng comments

8659f3d

tanujnay112 force-pushed the make_functions_great_again branch from e1c52e2 to 8659f3d Compare November 22, 2025 01:37

propel-code-bot bot reviewed Nov 22, 2025

View reviewed changes

rust/worker/src/execution/orchestration/compact.rs Show resolved Hide resolved

propel-code-bot bot reviewed Nov 22, 2025

View reviewed changes

rust/worker/src/execution/orchestration/compact.rs Show resolved Hide resolved

tanujnay112 merged commit 3fec52a into main Nov 22, 2025
62 checks passed

[ENH]: Execute task with no backfill or incremental #5867

[ENH]: Execute task with no backfill or incremental #5867

Conversation

tanujnay112 commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Test plan

Migration plan

Observability plan

Documentation Changes

Uh oh!

github-actions bot commented Nov 14, 2025

Reviewer Checklist

Testing, Bugs, Errors, Logs, Documentation

System Compatibility

Quality

Uh oh!

tanujnay112 commented Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

HammadB left a comment

Choose a reason for hiding this comment

Uh oh!

HammadB Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

tanujnay112 Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

HammadB Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

tanujnay112 Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HammadB Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

propel-code-bot bot Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Sicheng-Pan left a comment

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tanujnay112 commented Nov 14, 2025 •

edited

Loading

tanujnay112 commented Nov 14, 2025 •

edited

Loading