bitflicker64 commented on code in PR #3362:
URL: https://github.com/apache/hugegraph/pull/3362#discussion_r4306903938
##########
hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/HeartbeatService.java:
##########
@@ -200,10 +208,61 @@ protected void registerStore() {
this.storeInfo.setStoreAddress(IpUtil.getNearestAddress(options.getGrpcAddress()));
this.storeInfo.setRaftAddress(IpUtil.getNearestAddress(options.getRaftAddress()));
- long storeId = pdProvider.registerStore(this.storeInfo);
+ // Recovery is needed once per process, not when an established
stream reconnects.
+ boolean restoring = !restored;
+ this.storeInfo.setState(restoring ? Metapb.StoreState.Offline :
Metapb.StoreState.Up);
+ long storeId;
+ try {
+ storeId = pdProvider.registerStore(this.storeInfo);
+ } finally {
+ // Retry registration after a failure instead of heartbeating
an unregistered Store.
+ this.storeInfo.setState(Metapb.StoreState.Offline);
+ }
if (storeId != 0) {
storeInfo.setId(storeId);
storeMetadata.save(storeInfo);
+ if (restoring && storeEngine != null) {
+ this.clusterStats = pdProvider.getClusterStats();
+ timerNextDelay = REGISTER_RETRY_INTERVAL * 1000;
+ if (restoreTask == null) {
+ restoreTask = new FutureTask<>(() -> {
+ storeEngine.stateChanged(storeInfo,
Metapb.StoreState.Offline, Metapb.StoreState.Up);
+ return null;
+ });
+ restoreThread = new Thread(restoreTask,
"store-partition-restore");
+ restoreThread.start();
+ return;
+ }
+ if (!restoreTask.isDone()) {
+ return;
+ }
+ try {
+ restoreTask.get();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ return;
+ } catch (ExecutionException e) {
+ log.error("Store {} remains Offline after partition
restore failed", storeId, e);
+ restoreTask = null;
Review Comment:
Minor: A restore that keeps failing is retried in full about once a second,
with no backoff, for as long as the failure lasts.
After the `ExecutionException`, `restoreTask` is cleared, and
`timerNextDelay` is still the 1 s set at line 226. The next tick registers
Offline again and starts a new `store-partition-restore` thread. That thread
re-runs `loadPartition`, `syncPartitionsFromPD` and `createPartitionEngine` for
every partition. Each failed round logs this error, with one suppressed stack
trace per failed partition.
Separately, every tick during restore, failed or just slow, sends a full
`RegisterStore`. The Store logs `registerStore pd=...` at INFO
(`DefaultPdProvider` line 132). PD rewrites the Store record and the keepalive
entry and logs `Store register, id = ...` at INFO (`StoreNodeService.register`,
lines 209 to 229). The keepalive entry lives 300 s by default
(`store.keepAlive-timeout`), so renewing it every second is far more often than
needed.
Take one partition that cannot be restored, the case this PR deliberately
keeps Offline. The Store then logs the same set of stack traces every second
until someone fixes the data. Before this PR, the failure was logged once per
restore attempt, and attempts happened only at startup and on stream reconnects.
Requested change: back off restore retries after a failure, for example
doubling the delay up to `storeHBInterval`. Log the full aggregate trace only
on the first failure or when the set of failed partitions changes. While the
worker is still running, polling it every second is fine, but send the Offline
registration only every few seconds, well inside the keepalive timeout.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
