Murtadha Hubail has posted comments on this change.

Change subject: Allow lazy loading for persistent local resources
......................................................................


Patch Set 5:

(5 comments)

https://asterix-gerrit.ics.uci.edu/#/c/344/5/asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/opcallbacks/TempDatasetPrimaryIndexModificationOperationCallbackFactory.java
File 
asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/opcallbacks/TempDatasetPrimaryIndexModificationOperationCallbackFactory.java:

Line 68:     
> trim space.
Done


https://asterix-gerrit.ics.uci.edu/#/c/344/5/asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/resource/PersistentLocalResourceRepository.java
File 
asterix-transactions/src/main/java/edu/uci/ics/asterix/transaction/management/resource/PersistentLocalResourceRepository.java:

Line 48:     private final Cache<String, LocalResource> resourcesCache;
> s/resourcesCache/resourceCache?
I'm not sure if we have a naming standard. I think resourcesCache is better for 
readability :)


Line 205:     public HashMap<Long, LocalResource> loadAndGetAllResources() 
throws HyracksDataException {
> s/public/private?
RecoveryManager needs to have the complete map to use it during recovery.


Line 232:             //#. load all local resources. 
> trim space.
Done


Line 270:         return new ArrayList<LocalResource>(resourcesMap.values());
> Change the method signature here?
This method is part of ILocalResourceRepository. Both 
PersistentLocalResourceRepository and TransientLocalResourceRepository (in 
Hyracks) are implementing it. I didn't want to change the signature and break 
the TransientLocalResourceRepository. We could however add a new method called 
GetMaxResourceID in both of them, which will basically do the same but will be 
reading the resources one by one and updating a max. After that, to factor out 
the code for reading the resources, we will end up with the current 
implementation. Since all resources are loaded only during startup, there 
shouldn't be any memory problems.


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/344
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I48b9260a3280750145f6ddb3783673a299055910
Gerrit-PatchSet: 5
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Murtadha Hubail <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: Young-Seok Kim <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to