refactor(server): disable legacy master-worker scheduler logic#3082
Merged
Conversation
dosubot
Bot
added
size:XXL
This was referenced Jul 6, 2026
bitflicker64
force-pushed
the
fix/scheduler
branch
from
f0fbc4a to
abb09aa
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3082 +/- ##
============================================
- Coverage 36.80% 1.47% -35.33%
+ Complexity 338 21 -317
============================================
Files 805 783 -22
Lines 68587 65889 -2698
Branches 9029 8545 -484
============================================
- Hits 25241 975 -24266
- Misses 40653 64830 +24177
+ Partials 2693 84 -2609 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…precation behavior
…letes are recoverable When delete(force=true) is called and deleteFromDB() throws (e.g. result vertex removal fails), the task must be left as DELETING so cronSchedule() can retry. Previously the force-delete path skipped the status transition entirely, leaving the task as SUCCESS — unrecoverable.
…and review issues - Remove DELETING guard from TaskAndResultScheduler.save() — it blocked legitimate DELETING writes from updateStatus(), causing testTask:96 failure. The resurrect-race is already handled by HugeTask.cancel(). - Write DELETING status before deleteFromDB() in force-delete path so failed deletions are recoverable via cronSchedule(). Reuse existing task variable instead of re-querying DB. - Fix potential NPE in cronSchedule(): replace containsKey()+get() with get()+null-check on runningTasks ConcurrentHashMap. - Remove commented-out code and fix todo→TODO in close(). - Fix thread leak in TaskSchedulerServerInfoTest: wrap constructor in try block so executors are shut down on failure.
tasks(ImmutableList.of(id)) defaults to withResult=false and returns tasks without result, but the test expected the result field to be present. Pass explicit withResult=true to use queryTask(ids) path.
…i review fixes - GraphManager.initNodeRole(): add NullPointerException guard on role before toUpperCase() - StandardTaskScheduler.cancel(): overwrite task status to CANCELLED on successful cancel for consistency with DistributedTaskScheduler
- ignore deprecated task.scheduler_type for upgrade compatibility - avoid restoring legacy master-worker task statuses - harden distributed task deletion against save races - add focused task scheduler regression coverage
- avoid physically deleting locally running distributed tasks before runner exit - keep force delete on running tasks in DELETING and cancel flow - fail scheduler close when running tasks cannot drain in time - add hstore regression coverage for force deleting running tasks
- inject PD peers for hstore backend case-insensitively - keep queued task persistence after local queue admission - harden task cleanup and GraphManager test lifecycle - cover queue-full scheduling and mixed-case hstore config
- cancel locally running standard tasks before force deletion - tolerate missing distributed tasks during cancel retry - add regression coverage for delete and cancel races
imbajin
approved these changes
Jul 8, 2026
imbajin
left a comment
imbajin
left a comment
Member
There was a problem hiding this comment.
TODO: cleanup legacy code/configs in phrase2
imbajin
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR soft-disables the pre-PD master-worker task scheduling path while keeping old configs and task data upgrade-safe. Scheduler selection is now backend-driven:
hstoreusesDistributedTaskScheduler, and other backends use the localStandardTaskScheduler.Before -> After
Before:
task.scheduler_typeselected the scheduler mode.server_info, heartbeat, server load, and master/worker role election.SCHEDULING -> SCHEDULED -> QUEUED -> RUNNING, with the master assigning tasks to workers.After:
task.scheduler_typeis deprecated and ignored; old config files only emit a warning.QUEUED -> local executor, with no master-worker assignment.server_info.ServerInfoManagerand role-election startup are kept only as soft-disabled compatibility shims.Compatibility and Impact
SCHEDULING/SCHEDULEDtasks from the old master-worker path are preserved in storage but are not restored or executed automatically. Operators should drain or cancel old in-flight master-worker tasks before upgrading if those tasks still matter.server.role_election,server.id, andtask.scheduler_typeno longer control scheduling behavior.DELETINGis used as a retryable tombstone, running task deletion waits for the runner lifecycle, and save-after-delete resurrection is guarded.Follow-up Cleanup
This PR is the first soft-disable step, not the final physical deletion. Some deprecated config, role-election, and
server_infocode is intentionally left as compatibility scaffolding for this transition. After the soft-disable behavior is verified as stable, follow-up PRs should mark and remove the remaining dead paths, especially:server_infopersistence, heartbeat, load, and worker-picking code paths;The intended end state is to fully remove the old master-worker scheduling system after this compatibility window, while keeping the PD/HStore distributed scheduler as the only distributed scheduling path.