Steven Jacobs has posted comments on this change. Change subject: [ASTERIXDB-2386][CLUS] Allow extension of the global recovery manager ......................................................................
Patch Set 2: (8 comments) Addressed Comments. Uploading a new change. https://asterix-gerrit.ics.uci.edu/#/c/2640/2/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/CCApplication.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/CCApplication.java: PS2, Line 183: GlobalRecoveryManager > IGlobalRecoveryManager? Done PS2, Line 185: GlobalRecoveryManager > IGlobalRecoveryManager? Done Line 185: GlobalRecoveryManager globalRecoveryManager = > My extension knowledge is very limited, so please correct me if I'm wrong. This is the way that all of the extensions are handled. You can see the ideal data flow that you are mentioning happening in the call done by line 186. I changed the method name to "get" in the new patch. The RecoveryManager class is unaltered by this patch. It only affects whether the GlobalRecoveryManager can be extended. PS2, Line 186: GlobalRecoveryManager > not needed? Done Line 202: return new ExtensionProperties(PropertiesAccessor.getInstance(ccServiceCtx.getAppConfig())).getExtensions(); > Why this need to be changed? This is because the global recover managers is set up before the application context, and now the extensions need to be set up before the app context (lines 145-147 in the new change) https://asterix-gerrit.ics.uci.edu/#/c/2640/2/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/GlobalRecoveryManager.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/GlobalRecoveryManager.java: Line 132: MetadataManager.INSTANCE.getDataverse(mdTxnCtx, dataverse.getDataverseName()); > this is for? This one line is actually the whole fix for ASTERIXDB-2386. Basically the dataverses should be in the cache after recovery. https://asterix-gerrit.ics.uci.edu/#/c/2640/2/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/HyracksConnection.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/HyracksConnection.java: Line 124: > empty line. Done https://asterix-gerrit.ics.uci.edu/#/c/2640/2/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/IHyracksClientConnection.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/client/IHyracksClientConnection.java: PS2, Line 120: resetDeployedJobIdFactory > This method seems problematic as it exposes internals. I think that the cre Done -- To view, visit https://asterix-gerrit.ics.uci.edu/2640 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1213e702a77ededde18ee0b50bc105212f43480d Gerrit-PatchSet: 2 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Steven Jacobs <sjaco...@ucr.edu> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: Xikui Wang <xkk...@gmail.com> Gerrit-HasComments: Yes
