fix(store): rejoin a Store rebuilt empty at its old raft address - #3234
bitflicker64 wants to merge 1 commit into
Conversation
A Store whose data volume is lost comes back at the same raft address but registers with PD under a new store id. PD's reallocShards puts the new id into every shard group and fires ChangeShard, but the raft configuration already holds that address, so changePeers has nothing to add: the rebuilt Store never gets a raft node or a snapshot. The group leader still has the old id in its local shard group, and its partition heartbeat writes that id back to PD a few seconds later. Every group then names a store id that no longer exists and runs on two live replicas, while /v1/stores and the cluster state read healthy. When PD's shard list and the raft configuration have the same endpoints but PD names another store id at one of them, the leader now creates the raft node on that endpoint (jraft then installs a snapshot into it), takes PD's store ids into its local shard group, reports the group to PD and waits for the peer to catch up. Resolving a raft endpoint to a store id on a configuration commit also checks PD's current group, so a later leader change does not bring the old id back.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3234 +/- ##
============================================
- Coverage 41.35% 41.34% -0.01%
- Complexity 7299 7312 +13
============================================
Files 802 802
Lines 69688 69767 +79
Branches 9291 9309 +18
============================================
+ Hits 28816 28844 +28
- Misses 37576 37634 +58
+ Partials 3296 3289 -7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The repair path looks correct for the #3227 case. Endpoint equality is checked before any id sync, the leader is never included, createRaftNode is a no-op on a Store that already runs the group, and onLeaderStart and onStartFollowing both resolve through PD's group before the leader's next partition heartbeat, so the old id does not come back after a leader change. One minor retry issue: a failed createRaftNode now returns TASK_CONTINUE, and each retry takes a fresh raft snapshot. Evidence: read the full diff at 9b6bf41 with PartitionEngine.changePeers, doChangeShard, the SYNC_PARTITION_TASK handler, HgCmdClient.tryWithThrowable, HgCmdProcessor.handleCreateRaft, HgStoreEngine.createPartitionEngine and HeartbeatService.partitionHeartbeat. CI is green apart from codecov patch/project (patch coverage 30%) and build-server (hbase), which was still running when this was reviewed.
| if (!status.isOk()) { | ||
| log.info("Raft {} createRaftNode, peer:{}, reason:{}", getGroupId(), peer, | ||
| status.getErrorMsg()); | ||
| return HgRaftError.TASK_CONTINUE.toStatus(); |
There was a problem hiding this comment.
Minor: When createRaftNode fails here, the method returns TASK_CONTINUE, and doChangeShard re-queues the task straight away through addRaftTask(SYNC_PARTITION_TASK). Before that, local ids are not updated yet, so the retry finds the same changed endpoint and calls doSnapshot again on line 474. A single failure takes about 1.5 s: HgCmdClient.tryWithThrowable retries 5 times with a 100/200/300/400/500 ms backoff. Each retry also appends a raft entry, and setSnapshotLogIndexMargin is commented out in PartitionEngine.init, so jraft does not skip these snapshots. If the rebuilt Store goes down again after PD has moved the group to its new id (for example a crash-looping Pod), the leader of every affected group takes a snapshot and appends a log entry about every 1.5 to 2 s until the Store is back. changePeers handles the same createRaftNode failure at line 329 by returning the RPC status (code -1), which is not TASK_CONTINUE and does not re-queue. Please do the same here, or at least call doSnapshot only once per repair (for example after createRaftNode succeeds, or only when getReplicatorState shows the peer is not already installing a snapshot), so a peer that stays down does not trigger a retry loop that snapshots on every pass.
1 participant
Purpose of the PR
A Store whose data volume is lost comes back at the same raft address (on Kubernetes the StatefulSet always rebuilds the Pod under the same DNS name; on bare metal, any rebuild that reuses host:port) but registers with PD under a new store id. Retiring the old id per the documented procedure (
Tombstone, thenpatrolPartitions) never repairs the groups: every group ends up naming a store id that no longer exists, the rebuilt Store holds no partitions, every group runs on two live replicas, and/v1/stores, the cluster state, Hubble and Pod readiness all read healthy. #3227 has the full measurement and a diagram of the loop.There were two faults, and both had to go:
reallocShardsputs the new id Y into each group and fires ChangeShard. The group leader maps the ids to raft endpoints (shards2Peers), gets the same endpoint set it already has, sochangePeersadds nothing. The rebuilt Store never gets a raft node for any group, and jraft only installs a snapshot into a node that exists. On the [Bug] A Store rebuilt with an empty volume never rejoins its raft groups; retirement (Tombstone + patrol) cannot repair it and every health surface reads healthy #3227 run the leaders loggedReplicator stateChanged hugegraph-store-2...:8510 OFFLINEfor that address 11,814 times on store-0 alone.ShardGroupstill lists the old id X. Its partition heartbeat (HeartbeatService.partitionHeartbeat, every 5 s) sends that list, andPartitionService.partitionHeartbeatoverwrites PD's corrected group with it. On the [Bug] A Store rebuilt with an empty volume never rejoins its raft groups; retirement (Tombstone + patrol) cannot repair it and every health surface reads healthy #3227 run PD rewrote group 1 with X three seconds afterreallocShardswrote Y (Raft 1 updateShardGroup ... store_id: 1190848017232829764at 17:59:47, PD leader log).onConfigurationCommittedresolves endpoints through the same stale list, so any later leader change would do the same.Main Changes
All in
hg-store-core, Store side only. No PD, proto or configuration change.PartitionEngine.doChangeShard: afterchangePeers, call the newsyncShardIdentities. When PD's shard list and the raft configuration have the same endpoints but PD names another store id at one of them (never the leader itself), the leader:createRaftNodeto that endpoint with the current configuration, so the rebuilt Store creates the raft node and jraft installs the leader's snapshot into it (the same RPCchangePeersalready uses for new learners);Replicate(the existing wait loop, moved intowaitForReplicateso both paths share it). A timeout returnsTASK_CONTINUE, which the existing retry handles.No raft membership change happens: the endpoint is already a voter, and the rebuilt node joins that slot with an empty log.
PartitionManager.getStoreByRaftEndpoint(ShardGroup, Metapb.ShardGroup, String): when the id the local group holds for an endpoint is no longer in PD's current group, use the id PD's group has at that endpoint.onConfigurationCommittedfetches PD's group once and resolves through it, so a leader change after the repair does not put the old id back. The two-argument overload is unchanged.StoreIdChangeTest(inCoreSuiteTest): endpoint resolution when the local id is still valid, retired, or deleted from PD, and the endpoint-diff rule (finds a new id at an unchanged address, ignores real membership changes, never includes the leader itself).Not in this PR: PD retiring the old record by itself when a new id registers at its raft address. The documented
Tombstonestep is still needed to start the repair; with this change it now works. Readiness while a rebuilt Store catches up stays with #3229.Verifying these changes
mvn test -pl hugegraph-store/hg-store-test -am -P store-core-testruns 30 tests, 8 of them new, 0 failures (22 before).store-raftcore-test(9) andpd-core-test(104, 2 skipped) unchanged and green;editorconfig:checkclean. JDK 11, run 2026-09-24.Tombstonethe old id on the PD leader,patrolPartitions, wait up to 25 min. Same script, same chart, same host, only the Store/PD/Server images differ. Control: the:latestimages [Bug] A Store rebuilt with an empty volume never rejoins its raft groups; retirement (Tombstone + patrol) cannot repair it and every health surface reads healthy #3227 was measured on (apache master83ef9f3, storesha256:7bc0cbd81550). Fix: images built from this branch (9b6bf414, revision label checked in the pod).83ef9f3)9b6bf414):8520/v1/partition/{id}DELETE /v1/store/{old}/v1/storesand cluster stateUp; rebuilt StorepartitionCount0;Cluster_OKUp; rebuilt StorepartitionCount12;Cluster_OK(run 2, from 48 s on)Run 2 (same images, fresh cluster) adds a 5-minute health watch after convergence, a second
patrolPartitions, and a restart of the rebuilt Store with its PVC kept. It converged 17 s after the Tombstone. From the 48 s sample on, every 30 s sample readCluster_OK, the rebuilt StoreUpwithpartitionCount12, and all 12 groupsPState_Normal. The second patrol changed nothing. The restarted Store was Ready in 151 s under the same id, with 0 container restarts, nois illegalexit inloadPartitions, and all 12 groups served. The Store-levelpartitionCountfollows the Store heartbeat (30 s), so it lags the repair: run 1 sampled it 1 s after the Tombstone and sawpartitionCount0 andCluster_Warnwhile the snapshots were still installing.Store leader log on the fixed run, per group (store-0 and store-1 between them cover all 12):
Evidence (scripts, run logs, per-Store partition views, PD and Store logs, write ledgers): bitflicker64/hugegraph
logs/fix-3227(s2b-run.shis the harness,README.mdlists the layout, one folder per run)Does this PR potentially affect the following parts?
hg-store-core)Documentation Status
Doc - TODO: required documentation is pending; complete it before merging.Doc - Done: documentation is included here or linked below.Doc - No Need: no user-visible documentation is affected.Documentation files in this PR or paired hugegraph-doc PR:
The Helm chart README (#3218) currently tells operators never to delete a Store volume. That warning changes once this merges; see the list below.
After this merges
For whoever picks this up next. Nothing here blocks the merge.
masterat or after the merge commit of this PR, then build the images with the revision label:SHA=$(git rev-parse HEAD); IMAGE_TAG=m3227 SOURCE_REVISION=$SHA docker buildx bake -f docker/bake.hcl pd store server-hstore --set '*.platform=linux/amd64' --set "*.labels.org.opencontainers.image.revision=$SHA" --load, and tag each ashg3227/{pd,store,server}:master. (Or use the Docker Hub:latestpublished after the merge, and record its digests and revision instead of building.)OBSERVE_S=300 WAIT_S=1500 bin/s2b-run.sh masterwith the harness linked under Evidence. It expects the chart under~/hg-3227/chart/helm/hugegraph, andharness/kind.yaml,writer.shandverify-ledger.shfrom the evidence folder under~/hg-lifecycle(kind.yamlat the top, the two scripts inscripts/). It also needs the imageshg3227/hubble:controlandhg3227/probe:1, the latter built fromharness/probe.Dockerfile.converged=1; 0 groups naming the old id and 12 naming the new one; the rebuilt Store answers 200 on all 12:8520/v1/partition/{id}; after the observation window/v1/storesshows the rebuilt StoreUpwithpartitionCount12 and the clusterCluster_OK; the store-2 restart with its PVC comes back with 12 of 12 and nois illegalline;missing=0in the ledger. Attach thes2b.logandhealth.logto this PR as a comment.feat/hstore-helm-chart-3132, the head of apache PR feat(helm): add HStore deployment chart #3218 and its mirror feat(helm): add HStore deployment chart hugegraph/hugegraph#221, filehelm/hugegraph/README.md). Two places say a Store volume must never be deleted: the paragraph starting "Do not delete a Store's PersistentVolumeClaim on current images." under Disaster Recovery, and the first bullet of "## Limitations" ("A Store cannot be recovered in place after its volume is lost"). Replace them with the retirement procedure that now converges (find the old id at the replaced Pod's address,POST /v1/store/{oldId}with{"storeState":"Tombstone"}on the PD leader,patrolPartitions, verify every group lists onlyUpStores and the replaced Store's:8520/v1/partition/{id}answers 200, thenDELETE /v1/store/{oldId}). Keep a version caveat: images built before this PR's merge commit, including every published 1.7.x image and:latestbefore that merge, still cannot recover in place, and the old warning applies to them word for word. Only make this change after step 1 passes, and cite the step 1 run in the commit message.Upor add a readiness signal. It does change what [Feature] Store-side readiness for rolling replacement: registration marks Up before partition restore, and a stopped Store stays Up in every shard group for keepAlive-timeout #3229 is about: before, a Store rebuilt with an empty volume never received data, so "Up but empty" lasted forever; now it receives a snapshot for every group after the old id is retired, and the remaining gap is the window betweenUpand caught-up (1 s on the 243 MB test data, longer on real data), plus the staleUpof a stopped Store. Post one short comment on [Feature] Store-side readiness for rolling replacement: registration marks Up before partition restore, and a stopped Store stays Up in every shard group for keepAlive-timeout #3229 saying this, linking this PR.s2b-run.shfrom the evidence intoscripts/lifecycle/, add a row for it toscripts/lifecycle/README.md, and add a section toreferences/test-suite.mdunder Lifecycle with the procedure, the pass criteria from step 1, and the expected control result on images older than this merge (12 of 12 groups keep the old id, rebuilt Store 500 on every group,Cluster_OKthroughout). Mark the old S2b finding in that file as fixed by this PR.