Repository navigation
docs(pd): document the periodic leader balance settings - #547
arshilkxwork wants to merge 4 commits into
Conversation
apache/hugegraph#3356 adds pd.balance-leader-enable, pd.balance-leader-interval and pd.balance-leader-max-skew. Add them to the PD core table on the PD quickstart page, EN and CN, with a note on when a check or a transfer is skipped.
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Score: 9.5/10. Summary: The pd.balance-leader-enable, pd.balance-leader-interval and pd.balance-leader-max-skew rows and the note under the table match apache/hugegraph#3356 at f8c3a952f (PDConfig defaults, distribution application.yml, the init() validation warning, and the skip and transfer checks in TaskScheduleService), and the EN and CN pages agree. This should merge after apache/hugegraph#3356, which is still open; if it changes before merging, these rows need another check.
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The leader transfer lag limit is described as a guarantee, but the implementation only estimates it. Evidence: TaskScheduleService.canTakeLeader uses asynchronously sampled heartbeats; testAutoBalancePartitionLeaderWithEarlyBurst demonstrates an underestimated gap.
| > [!NOTE] | ||
| > The `pd.balance-leader-*` settings are introduced in 1.8.0 (`master` branch). With `pd.balance-leader-enable` turned on, PD periodically balances partition leaders across Stores: | ||
| > - A balance check is skipped if a shard group is unhealthy, lacks recent heartbeats, is undergoing membership changes, or holds a shard on an inactive Store. | ||
| > - Planned transfers are only scheduled to Stores whose state is `Up` and whose replication log is up to date (within 1000 applied entries of the leader). |
There was a problem hiding this comment.
canTakeLeader estimates the gap from asynchronous Store heartbeats. The paired implementation’s testAutoBalancePartitionLeaderWithEarlyBurst shows a replica actually 1500 entries behind can pass the check, potentially delaying writes during the transfer. Please describe the threshold as a best-effort estimate and clarify the same limitation in the Chinese page.
There was a problem hiding this comment.
Thanks, fixed in cb41bf4. The bullet now calls the 1000-entry limit an estimate: PD works it out from Store heartbeats sent at different times, so a burst of writes can hold back a caught-up target or let a lagging one through. In the lagging case, writes to the partition can stall until the target catches up or the transfer times out. The Chinese page says the same.
PD estimates the gap from Store heartbeats sent at different times, so a burst of writes can let through a target that is more than 1000 entries behind. Say so on the EN and CN pages, and say that writes to the partition can then stall until the target catches up or the transfer times out.
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Score: 9/10. Summary: The three pd.balance-leader-* rows and the note under the table match apache/hugegraph#3356 at f8c3a952f, including the best-effort wording for the 1000-entry lag check, and the EN and CN pages agree. Merge this after apache/hugegraph#3356, which is still open, since master has none of these settings yet.
Docs for apache/hugegraph#3356 (apache/hugegraph#3356), which adds optional periodic partition leader balancing to PD with three new settings.
Updated in both EN and CN, in the PD core table of
quickstart/hugegraph/hugegraph-pd.md:pd.balance-leader-enable(false): run leader balancing on a timer on the PD leader.pd.balance-leader-interval(300seconds): time between checks, and the minimum time between two plans, counted from the last plan including a manualbalanceLeaderscall.pd.balance-leader-max-skew(2): the leader-count spread above which a check balances.A short paragraph under the table says the keys exist only in versions after 1.7.0, when a check or a single transfer is skipped, and that an invalid interval or skew leaves the feature off with a warning.
The text matches apache/hugegraph#3356 at f8c3a952f. This should merge after that PR, since no released PD has these settings yet.
I ran
git diff --check. I did not build the site locally.