Xikui Wang has posted comments on this change.

Change subject: [ASTERIXDB-2386][CLUS] Allow extension of the global recovery 
manager
......................................................................


Patch Set 2:

(4 comments)

Added several minor comments. It would be better for someone who is familiar 
with recovery to take a look...

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:

Line 185:         GlobalRecoveryManager globalRecoveryManager =
My extension knowledge is very limited, so please correct me if I'm wrong. 
IMHO, I think the way for creating the GlobalRecoveryManager is not very ideal. 
I think an ideal flow would be: if there is no extension, create the default 
one; if there is an extension, let extension create one. The master relies on 
extension mode feels strange to me. The names of the two methods are confusing 
too. It feels more like a configuration but not a creation to me... Also, is 
recoveryManager still the same after the patch?


Line 202:         return new 
ExtensionProperties(PropertiesAccessor.getInstance(ccServiceCtx.getAppConfig())).getExtensions();
Why this need to be changed?


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?


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.


-- 
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 <[email protected]>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Xikui Wang <[email protected]>
Gerrit-HasComments: Yes

Reply via email to