You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "contrueCT (via GitHub)" <gi...@apache.org> on 2026/06/17 13:48:31 UTC
[PR] fix(server): avoid loading huge task results for metadata queries [hugegraph]
contrueCT opened a new pull request, #3060:
URL: https://github.com/apache/hugegraph/pull/3060
<!--
Thank you very much for contributing to Apache HugeGraph, we are happy that you want to help us improve it!
Here are some tips for you:
1. If this is your first time, please read the [contributing guidelines](https://github.com/apache/hugegraph/blob/master/CONTRIBUTING.md)
2. If a PR fix/close an issue, type the message "close xxx" (xxx is the link of related
issue) in the content, GitHub will auto link it (Required)
3. Name the PR title in "Google Commit Format", start with "feat | fix | perf | refactor | doc | chore",
such like: "feat(core): support the PageRank algorithm" or "fix: wrong break in the compute loop" (module is optional)
skip it if you are unsure about which is the best component.
4. One PR address one issue, better not to mix up multiple issues.
5. Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]` (or click it directly after
published)
-->
## Purpose of the PR
Fix task metadata/recovery paths that can still load and decompress large task results, making `GET /tasks`, task restore, and `DELETE /tasks/{id}?force=true` unusable when historical tasks contain huge result payloads.
This is related to #3057 and #3059. The PR focuses on metadata-only access and cleanup paths, while large Gremlin result export/chunking can be handled separately.
<!-- or use "fix #xxx", "xxx" is the ID-link of related issue, e.g: close #1024 -->
<!--
Please explain more context in this section, clarify why the changes are needed.
e.g:
- If you propose a new API, clarify the use case for a new API.
- If you fix a bug, you can clarify why it is a bug, and should be associated with an issue.
-->
## Main Changes
- Add scheduler APIs that allow task reads with or without result payloads.
- Add metadata-only task deserialization that avoids reading `~task_result`.
- Keep `GET /tasks/{id}` default behavior compatible, and add `with_result=false`.
- Make `GET /tasks`, task restore, and delete paths use metadata-only reads.
- Add lightweight task vertex deletion to avoid `GraphTransaction.removeVertex()` force-loading indexed task vertices.
- Avoid loading separated `~taskresult` vertices when cleaning distributed task results.
- Return a result-stripped copy for in-memory tasks when `withResult=false`.
<!-- Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. These change logs are helpful for better and faster reviews.)
For example:
- If you introduce a new feature, please show detailed design here or add the link of design documentation.
- If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
- If there is a discussion in the mailing list, please add the link. -->
## Verifying these changes
<!-- Please pick the proper options below -->
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests here)*.
- [x] Need tests and can be verified as follows:
- `mvn test -pl hugegraph-server/hugegraph-test -am -P core-test,rocksdb -Dtest=TaskCoreTest -DfailIfNoTests=false -Dcheckstyle.skip=true -Drat.skip=true`
- `mvn compile -pl hugegraph-server/hugegraph-api -am -DskipTests=true -Dcheckstyle.skip=true -Drat.skip=true`
- `mvn test-compile -pl hugegraph-server/hugegraph-test -am -P api-test,rocksdb -Dtest=TaskApiTest -DskipTests=true -Dcheckstyle.skip=true -Drat.skip=true`
## Does this PR potentially affect the following parts?
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] Dependencies ([add/update license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses) info & [regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh)) <!-- Don't forget to add/update the info in "LICENSE" & "NOTICE" files (both in root & dist module) -->
- [ ] Modify configurations
- [ ] The public API
- [ ] Other affects (typed here)
- [x] Nope
## Documentation Status
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `Doc - TODO` <!-- Your PR changes impact docs and you will update later -->
- [ ] `Doc - Done` <!-- Related docs have been already added or updated -->
- [x] `Doc - No Need` <!-- Your PR changes don't impact/need docs -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): avoid loading huge task results for metadata queries [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #3060:
URL: https://github.com/apache/hugegraph/pull/3060#discussion_r3447643828
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:
##########
@@ -306,10 +306,13 @@ protected <V> HugeTask<V> deleteFromDB(Id id) {
Iterator<Vertex> vertices = this.tx().queryTaskInfos(id);
HugeVertex vertex = (HugeVertex) QueryResults.one(vertices);
if (vertex == null) {
+ this.deleteTaskResultFromTx(id);
return null;
}
- HugeTask<V> result = HugeTask.fromVertex(vertex);
- this.tx().removeVertex(vertex);
+ HugeTask<V> result = HugeTask.fromVertex(vertex, false);
+ // Prefer a retryable orphan result over a live task missing result.
+ this.tx().removeTaskVertex(vertex);
Review Comment:
‼️ **Make orphan result cleanup reachable from production retries**
This failure mode is now explicitly tested, but the recovery path in the test calls `deleteFromDBForTest(id)` directly after the task vertex has already been removed. In production, `cronSchedule()` only discovers delete work by scanning `DELETING` task vertices, while `TaskAndResultTransaction` is auto-commit and `removeTaskVertex()` commits through `afterWrite()`. If `deleteTaskResultFromTx()` fails after this line, the remaining `~taskresult` vertex has no task vertex/status index entry left for the scheduler to rediscover, so the large result can leak permanently. Please either keep a retryable tombstone until result cleanup succeeds, add a production orphan-result cleanup path, or keep the task vertex until the result delete has succeeded; the regression test should exercise that production recovery path rather than only a direct helper call.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskAndResultScheduler.java:
##########
@@ -103,41 +103,65 @@ public <V> void save(HugeTask<V> task) {
}
@Override
- public <V> HugeTask<V> task(Id id) {
+ public <V> HugeTask<V> task(Id id, boolean withResult) {
HugeTask<V> task = this.call(() -> {
Iterator<Vertex> vertices = this.tx().queryTaskInfos(id);
Vertex vertex = QueryResults.one(vertices);
if (vertex == null) {
return null;
}
- return HugeTask.fromVertex(vertex);
+ return HugeTask.fromVertex(vertex, withResult);
});
if (task == null) {
throw new NotFoundException("Can't find task with id '%s'", id);
}
- HugeTaskResult taskResult = queryTaskResult(id);
- if (taskResult != null) {
- task.result(taskResult);
+ if (withResult) {
+ HugeTaskResult taskResult = queryTaskResult(id);
+ if (taskResult != null) {
+ task.result(taskResult);
+ }
}
return task;
}
@Override
public <V> Iterator<HugeTask<V>> tasks(List<Id> ids) {
- return this.tasksWithoutResult(ids);
+ return this.tasks(ids, true);
Review Comment:
⚠️ **Preserve metadata-only defaults for bulk task scans**
Before this PR, the task/result-backed scheduler's no-flag `tasks(ids)` and `tasks(status, limit, page)` overloads returned metadata-only tasks; the result-loading path was only in the protected `queryTask(...)` helpers. This now routes legacy bulk callers through `withResult=true`, which means any caller not audited in this PR can again load/decompress historical huge task results. Please keep the no-flag bulk overloads metadata-only for this scheduler and require callers that need payloads to opt in through the new `withResult=true` overloads; add a regression covering the no-flag bulk behavior so this boundary stays explicit.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskScheduler.java:
##########
@@ -47,12 +47,25 @@ public interface TaskScheduler {
<V> HugeTask<V> delete(Id id, boolean force);
- <V> HugeTask<V> task(Id id);
+ default <V> HugeTask<V> task(Id id) {
+ return this.task(id, true);
+ }
- <V> Iterator<HugeTask<V>> tasks(List<Id> ids);
+ <V> HugeTask<V> task(Id id, boolean withResult);
Review Comment:
⚠️ **Keep existing TaskScheduler implementers source-compatible**
The old no-argument methods were changed to defaults, but the new `withResult` overloads are abstract. Any existing downstream `TaskScheduler` implementation that compiled against the old interface now has to add three new methods before it can recompile, even if it does not support separated task results. Please make the new overloads default methods that delegate to the old methods, and let only the built-in schedulers override them where `withResult=false` is meaningful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): avoid loading huge task results for metadata queries [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #3060:
URL: https://github.com/apache/hugegraph/pull/3060#discussion_r3430871241
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/TaskCoreTest.java:
##########
@@ -115,6 +116,79 @@ public void testTask() throws TimeoutException {
});
}
+ @Test
+ public void testTaskWithoutResult() throws TimeoutException {
+ HugeGraph graph = graph();
+ TaskScheduler scheduler = graph.taskScheduler();
+ CountDownLatch latch = new CountDownLatch(1);
+
+ TaskCallable<String> callable = new TaskCallable<String>() {
Review Comment:
⚠️ **Use a reloadable task callable in this regression test**
This anonymous `TaskCallable` is persisted by class name, then the worker reloads it through `TaskCallable.fromClass()`. It has no public no-arg constructor, so the task fails to load as `TaskCallable$1`, and `scheduler.waitUntilTaskCompleted(id, 10)` times out. I reproduced it with `mvn test -pl hugegraph-server/hugegraph-test -am -P unit-test -Dtest=TaskCoreTest#testTaskWithoutResult -DfailIfNoTests=false -ntp`. Please use an existing reloadable callable or add a small static callable class for this test.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskAndResultScheduler.java:
##########
@@ -103,41 +103,65 @@ public <V> void save(HugeTask<V> task) {
}
@Override
- public <V> HugeTask<V> task(Id id) {
+ public <V> HugeTask<V> task(Id id, boolean withResult) {
HugeTask<V> task = this.call(() -> {
Iterator<Vertex> vertices = this.tx().queryTaskInfos(id);
Vertex vertex = QueryResults.one(vertices);
if (vertex == null) {
return null;
}
- return HugeTask.fromVertex(vertex);
+ return HugeTask.fromVertex(vertex, withResult);
});
if (task == null) {
throw new NotFoundException("Can't find task with id '%s'", id);
}
- HugeTaskResult taskResult = queryTaskResult(id);
- if (taskResult != null) {
- task.result(taskResult);
+ if (withResult) {
+ HugeTaskResult taskResult = queryTaskResult(id);
+ if (taskResult != null) {
+ task.result(taskResult);
+ }
}
return task;
}
@Override
public <V> Iterator<HugeTask<V>> tasks(List<Id> ids) {
- return this.tasksWithoutResult(ids);
+ return this.tasks(ids, false);
Review Comment:
‼️ **Preserve the existing `tasks(...)` result contract**
The new `TaskScheduler` default methods keep the old behavior as `tasks(ids) -> tasks(ids, true)`, but this override changes the task/result-backed scheduler to return metadata-only tasks by default. Existing Java callers of `scheduler.tasks(...).next().result()` can now silently get `null` while `scheduler.task(id)` still returns the result. Please keep the no-result path opt-in by removing these overrides or delegating them to `withResult=true`, and pass `false` only from the metadata-only call sites such as REST list, restore, delete, and scheduler scans.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): avoid loading huge task results for metadata queries [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin merged PR #3060:
URL: https://github.com/apache/hugegraph/pull/3060
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): avoid loading huge task results for metadata queries [hugegraph]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3060:
URL: https://github.com/apache/hugegraph/pull/3060#issuecomment-4731307836
## [Codecov](https://app.codecov.io/gh/apache/hugegraph/pull/3060?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `59.84252%` with `51 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 29.90%. Comparing base ([`c3f56b5`](https://app.codecov.io/gh/apache/hugegraph/commit/c3f56b5e9f26dddab7201da9a14ecd9d8c869eb2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`47fb2f9`](https://app.codecov.io/gh/apache/hugegraph/commit/47fb2f95f8e335f20b85b37e89896dd8a67d9455?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/hugegraph/pull/3060?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../apache/hugegraph/task/TaskAndResultScheduler.java](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Ftask%2FTaskAndResultScheduler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL3Rhc2svVGFza0FuZFJlc3VsdFNjaGVkdWxlci5qYXZh) | 0.00% | [26 Missing :warning: ](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...g/apache/hugegraph/task/StandardTaskScheduler.java](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Ftask%2FStandardTaskScheduler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL3Rhc2svU3RhbmRhcmRUYXNrU2NoZWR1bGVyLmphdmE=) | 70.37% | [7 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../main/java/org/apache/hugegraph/task/HugeTask.java](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Ftask%2FHugeTask.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL3Rhc2svSHVnZVRhc2suamF2YQ==) | 86.84% | [2 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...pache/hugegraph/task/DistributedTaskScheduler.java](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Ftask%2FDistributedTaskScheduler.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL3Rhc2svRGlzdHJpYnV0ZWRUYXNrU2NoZWR1bGVyLmphdmE=) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ava/org/apache/hugegraph/task/TaskTransaction.java](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Ftask%2FTaskTransaction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL3Rhc2svVGFza1RyYW5zYWN0aW9uLmphdmE=) | 76.47% | [1 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../org/apache/hugegraph/auth/HugeGraphAuthProxy.java](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Fauth%2FHugeGraphAuthProxy.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWdlZ3JhcGgvYXV0aC9IdWdlR3JhcGhBdXRoUHJveHkuamF2YQ==) | 57.14% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ain/java/org/apache/hugegraph/api/job/TaskAPI.java](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Fapi%2Fjob%2FTaskAPI.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWdlZ3JhcGgvYXBpL2pvYi9UYXNrQVBJLmphdmE=) | 80.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/hugegraph/pull/3060?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (c3f56b5) and HEAD (47fb2f9). Click for more details.
>
> <details><summary>HEAD has 2 uploads less than BASE</summary>
>
>| Flag | BASE (c3f56b5) | HEAD (47fb2f9) |
>|------|------|------|
>||3|1|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3060 +/- ##
============================================
- Coverage 36.10% 29.90% -6.20%
+ Complexity 338 264 -74
============================================
Files 803 804 +1
Lines 68291 68382 +91
Branches 8970 8982 +12
============================================
- Hits 24656 20452 -4204
- Misses 40990 45564 +4574
+ Partials 2645 2366 -279
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/hugegraph/pull/3060?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): avoid loading huge task results for metadata queries [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #3060:
URL: https://github.com/apache/hugegraph/pull/3060#discussion_r3446639600
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/core/TaskCoreTest.java:
##########
@@ -115,6 +115,88 @@ public void testTask() throws TimeoutException {
});
}
+ @Test
+ public void testTaskWithoutResult() throws TimeoutException {
+ HugeGraph graph = graph();
+ TaskScheduler scheduler = graph.taskScheduler();
+
+ Id id = IdGenerator.of(88889);
+ HugeTask<?> task = new HugeTask<>(id, null, new SleepCallable<>());
+ task.type("test");
+ task.name("metadata-task-in-memory");
+ scheduler.schedule(task);
+
+ try {
+ Whitebox.setInternalState(task, "result", "\"in-memory-result\"");
+
+ HugeTask<?> taskWithoutResult = scheduler.task(id, false);
+ Assert.assertEquals("metadata-task-in-memory",
+ taskWithoutResult.name());
+ Assert.assertNull(taskWithoutResult.result());
+
+ Iterator<HugeTask<Object>> iter = scheduler.tasks(ImmutableList.of(id),
+ false);
+ Assert.assertTrue(iter.hasNext());
+ taskWithoutResult = iter.next();
+ Assert.assertEquals("metadata-task-in-memory",
+ taskWithoutResult.name());
+ Assert.assertNull(taskWithoutResult.result());
+ Assert.assertFalse(iter.hasNext());
+ } finally {
+ Whitebox.setInternalState(task, "result", null);
+ }
+
+ scheduler.waitUntilTaskCompleted(id, 10);
+ scheduler.delete(id, false);
+
+ id = IdGenerator.of(88890);
+ task = new HugeTask<>(id, null, new MetadataResultCallable());
+ task.type("test");
+ task.name("metadata-task");
+ scheduler.schedule(task);
+
+ scheduler.waitUntilTaskCompleted(id, 10);
+
+ HugeTask<?> taskWithResult = scheduler.task(id, true);
+ Assert.assertEquals("\"metadata-result\"", taskWithResult.result());
+
Review Comment:
This currently proves the `withResult` switch for a small result, but it does not reproduce the original failure mode: loading/decompressing a large historical task result. Please add a regression case with a genuinely large/compressed result and assert that metadata-only reads (`task(id, false)`, `tasks(..., false)`, and ideally restore/scheduler metadata paths) do not touch `task_result`.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java:
##########
@@ -308,8 +308,9 @@ protected <V> HugeTask<V> deleteFromDB(Id id) {
if (vertex == null) {
return null;
}
- HugeTask<V> result = HugeTask.fromVertex(vertex);
- this.tx().removeVertex(vertex);
+ HugeTask<V> result = HugeTask.fromVertex(vertex, false);
+ this.deleteTaskResultFromTx(id);
Review Comment:
Please make the cleanup expectation explicit here. This path now deletes `~taskresult` before deleting `~task`, but `TaskTransaction`/`TaskAndResultTransaction` run with auto-commit and `removeTaskVertex()` commits in `afterWrite()`. If the second delete fails after this call succeeds, we can leave a live task whose result has already been removed. Could you either make the two removals failure-safe/atomic, or add a targeted test/explanation showing why this intermediate state is acceptable/recoverable?
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/job/TaskAPI.java:
##########
@@ -136,12 +137,17 @@ public Map<String, Object> get(@Context GraphManager manager,
@Parameter(description = "The graph name")
@PathParam("graph") String graph,
@Parameter(description = "The task id")
- @PathParam("id") long id) {
+ @PathParam("id") long id,
+ @Parameter(description = "Whether to load task result")
+ @DefaultValue("true")
Review Comment:
Since this introduces a new public query parameter, please update the PR checklist/docs status accordingly, or briefly explain why the generated OpenAPI annotation is sufficient and no user-facing docs are needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org
Re: [PR] fix(server): avoid loading huge task results for metadata queries [hugegraph]
Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on PR #3060:
URL: https://github.com/apache/hugegraph/pull/3060#issuecomment-4758480678
Thanks, the current head looks directionally good and preserves the default `GET /tasks/{id}` behavior. Could you please add or clarify a bit more regression coverage before merge?
Two areas seem worth covering:
1. A regression case with a genuinely large/compressed task result, verifying metadata-only paths such as `GET /tasks`, task restore/scheduler loops, and `with_result=false` do not load/decompress `task_result`.
2. The distributed task/result delete path: please either make the `~task` + `~taskresult` cleanup failure-safe/atomic, or add a targeted test/explanation for the current two-step delete path. Since `TaskTransaction` auto-commits after `removeTaskVertex()`, a failure between deleting `~taskresult` and deleting `~task` could otherwise leave a live task whose result has already been removed.
Minor checklist note: `with_result` is a new query parameter, so the PR checklist/docs status may need to reflect that public API surface, even if the runtime OpenAPI annotation already exposes it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org