Skip to content

feat: Add task result chunking support (issue #3071) - #3127

Open
cyforkk wants to merge 1 commit into
apache:masterfrom
cyforkk:task-result-chunking
Open

feat: Add task result chunking support (issue #3071)#3127
cyforkk wants to merge 1 commit into
apache:masterfrom
cyforkk:task-result-chunking

Conversation

@cyforkk

@cyforkk cyforkk commented Jul 29, 2026

Copy link
Copy Markdown

Summary

This PR adds support for chunking large task results to avoid memory overflow.

Changes

  • Added TASK_RESULT_CHUNK_SIZE config option to CoreOptions.java (default 1MB)
  • Added chunkKey() and isChunkedProperty() helper methods to P class
  • Implemented chunked write in asArray() method
  • Added RESULT_CHUNK_COUNT constant

Usage

When the compressed result exceeds task.result_chunk_size, it will be split into
multiple properties (~task_result_0, ~task_result_1, ...).

Set task.result_chunk_size=0 to disable chunking (preserves original behavior).

Related

Part of issue #3071 (Chunk 1-4)

🤖 Generated with Claude Code

- Add TASK_RESULT_CHUNK_SIZE config option to CoreOptions
- Add chunkKey() and isChunkedProperty() helper methods to P class
- Implement chunked write in asArray() method
- Add RESULT_CHUNK_COUNT constant

Co-Authored-By: Claude <noreply@anthropic.com>
@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. feature New feature labels Jul 29, 2026
@imbajin
imbajin requested a review from Copilot July 29, 2026 22:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to add configurable chunking for large Gremlin task results in hugegraph-core, splitting compressed task result blobs into smaller pieces to avoid exceeding per-property storage limits.

Changes:

  • Introduces task.result_chunk_size (CoreOptions.TASK_RESULT_CHUNK_SIZE) with a 1MB default.
  • Updates HugeTask.asArray() to optionally split compressed ~task_result into chunk properties and write a chunk-count marker.
  • Adds helper constants/methods in HugeTask.P for chunk key naming and identification.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/HugeTask.java Adds chunked result property writing and chunk-related helpers/constants.
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/config/CoreOptions.java Adds the task.result_chunk_size configuration option.
Comments suppressed due to low confidence (2)

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/HugeTask.java:590

  • checkPropertySize(chunk.length, P.chunkKey(i)) uses the default limit BytesBuffer.MAX_PROPERTIES (65,535) for non-RESULT keys, so with the default 1MB chunk size this will always throw LimitExceedException and chunking can never succeed. Validate against BytesBuffer.BYTES_LEN_MAX (or rely on the config range) instead of checkPropertySize() here.
                    byte[] chunk = new byte[end - start];
                    System.arraycopy(bytes, start, chunk, 0, end - start);
                    checkPropertySize(chunk.length, P.chunkKey(i));
                    list.add(P.chunkKey(i));
                    list.add(chunk);

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/HugeTask.java:594

  • Writing chunked results as ~task_result_0, ~task_result_1, … plus ~task_result_chunk_count will break both persistence and loading: (1) TaskTransaction.initProperties() only creates the ~task_result property key for the task vertex label, so these new keys are not in schema; (2) HugeTask.property() only handles P.RESULT and throws AssertionError for unknown keys, so fromVertex(..., withResult=true) will fail when it encounters the chunk keys/count. This needs a schema/model change (e.g., store chunks under a single multi-valued property or add explicit support for these keys in schema + deserialization).
                    list.add(P.chunkKey(i));
                    list.add(chunk);
                }
                // Write chunk count
                list.add(P.RESULT_CHUNK_COUNT);
                list.add(String.valueOf(numChunks));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

checkPropertySize(bytes.length, P.RESULT);
list.add(P.RESULT);
list.add(bytes);
int chunkSize = CoreOptions.instance().get(CoreOptions.TASK_RESULT_CHUNK_SIZE);
new ConfigOption<>(
"task.result_chunk_size",
"The chunk size for task results in bytes. Set 0 to disable chunking.",
rangeInt(0, BytesBuffer.BYTES_LEN_MAX),

@imbajin imbajin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: yes. Summary: The chunking implementation does not compile and is not wired into the task schema, read path, or distributed result-storage path. Evidence: exact-head CI compilation errors and static traces through HugeTask, TaskTransaction, StandardHugeGraph, and TaskAndResultScheduler.

list.add(bytes);
int chunkSize = CoreOptions.instance().get(CoreOptions.TASK_RESULT_CHUNK_SIZE);

if (chunkSize > 0 && bytes.length > chunkSize) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ This is not the shared persisted-result path. TaskAndResultScheduler.save() stores task metadata through asArrayWithoutResult() and writes the result through unchanged HugeTaskResult.asArray(), so HStore/distributed deployments still persist one BLOB; HugeTask.set() also enforces the total result limit before this branch runs. Please center chunking in the actual result-storage abstraction and cover both local and distributed persistence, including old-format compatibility.

@contrueCT

Copy link
Copy Markdown
Contributor

Thanks for the implementation. I reviewed the current patch, and it still seems to follow the original design that I raised concerns about in issue #3071.

The main problem is that the chunking logic is still implemented in HugeTask.asArray(). At this point, the result has already been fully serialized, and HugeTask.set() has already applied the total result-size check, so an oversized result may still fail before the chunking logic is reached.

More importantly, this is no longer the shared persisted-result path. TaskAndResultScheduler.save() stores task metadata through HugeTask.asArrayWithoutResult() and persists the actual result separately through HugeTaskResult.asArray(). Since HugeTaskResult is unchanged in this PR, distributed/HStore deployments will still store the result as a single BLOB, and the new chunking logic in HugeTask.asArray() may not be used for the actual result at all.

The current patch also only adds the chunked write path. There is no corresponding schema update or read/reassembly support for ~task_result_0, ~task_result_1, and ~task_result_chunk_count, so even the legacy HugeTask persistence path would not be complete.

I think the design should first be revised around the actual result-storage abstraction, most likely HugeTaskResult, and clearly define:

  • how chunked and legacy single-BLOB results are distinguished;
  • how task.result_size_limit applies to the total result versus each chunk;
  • how chunks are read and reassembled safely;
  • how local and distributed task schedulers share the same behavior;
  • how old persisted task results remain readable.

Tests should cover the real persisted-result path, distributed/HStore behavior, chunk reassembly, incomplete chunks, and backward compatibility with the old single-property format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants