imbajin commented on code in PR #3362:
URL: https://github.com/apache/hugegraph/pull/3362#discussion_r4307909612
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/StoreNodeService.java:
##########
@@ -206,13 +198,27 @@ public Metapb.Store register(Metapb.Store store) throws
PDException {
}
}
+ // Updated Stores register Offline to obtain their identity before
restoring partitions.
+ // Older Stores still request Up, and Pending activation and duplicate
checks stay intact.
+ if (restoring && storeState == Metapb.StoreState.Up) {
+ storeState = Metapb.StoreState.Offline;
+ store = store.toBuilder().setState(storeState).build();
+ }
+
// Store information
storeInfoMeta.updateStore(store);
if (storeState == Metapb.StoreState.Up) {
// Update the store active status
storeInfoMeta.keepStoreAlive(store);
onStoreStatusChanged(store, Metapb.StoreState.Offline,
Metapb.StoreState.Up);
checkStoreStatus();
+ } else if (restoring && storeState == Metapb.StoreState.Offline) {
+ // A returned Store is alive even while its engines recover. Renew
on each retry.
+ storeInfoMeta.keepStoreAlive(store);
Review Comment:
‼️ Critical: This branch renews a restoring Store's TTL-backed active entry
while its canonical state is Offline. `StoreInfoMeta.getActiveStores()` returns
those TTL entries without filtering StoreState, and `allocShards()` consumes
them directly, so PD can assign new shard replicas to this Store before its
engines finish restoring. Please separate the restart liveness lease from
placement eligibility, or exclude Offline Stores from placement until
restoration succeeds.
##########
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/service/PDService.java:
##########
@@ -438,6 +439,24 @@ public void getStore(Pdpb.GetStoreRequest request,
observer.onCompleted();
}
+ @Override
+ public void reportStoreShutdown(Pdpb.SetStoreRequest request,
+ StreamObserver<Pdpb.SetStoreResponse>
observer) {
+ if (!isLeader()) {
+ redirectToLeader(PDGrpc.getReportStoreShutdownMethod(), request,
observer);
+ return;
+ }
+ Pdpb.SetStoreResponse response;
+ try {
+ Metapb.Store store =
storeNodeService.reportStoreShutdown(request.getStore().getId());
Review Comment:
⚠️ Important: This RPC trusts the Store ID supplied by the caller and marks
that Store Offline without checking that the caller owns it.
`GRpcServerConfig.configure()` does not register `GrpcAuthentication`, so any
client that can reach PD can bypass `SetStore(Offline)`'s readiness check and
change another Store's state. Please authenticate the reporting Store and
require its identity to match the target, or restrict this RPC to an explicit
privileged caller.
--
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]
