Till Westmann has posted comments on this change. Change subject: Add support for persistent local resources to IndexLifecycleManager ......................................................................
Patch Set 2: I have a few thoughts, but not necessarily a concrete suggestion (as I don't really understand the requirements here). 1) It seems that longs are a nice and efficient way to identify resources. And we generally seem to use maps to map between these longs and the actual resources. But that doesn't seem to be feasible here. It would be nice to understand why. 2) If the resource identifier would be changed to be an Object, it could either be a (boxed) Long or a String, so the client source code using longs would not have to change. However other implementations of the interface should be changed. Also this might just keep the interface small and create some ugly type-based handling behind the interface. 3) If it actually turns out that Strings are the better identifiers here for everything, we should probably deprecate the other methods such that the other clients migrate away from those interfaces and we can tighten the interface again. 4) I'm concerned about additional maintenance burden introduced by the client code that takes decisions based on "lcManager.supportsPersistentLocalResources()". It would be nice if such decisions (if necessary) would be taken in one reusable piece of code and not by every client of the interface. Does this make sense? -- To view, visit https://asterix-gerrit.ics.uci.edu/343 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2e8e974fc2f746959639ce94351f8e419a7f9093 Gerrit-PatchSet: 2 Gerrit-Project: hyracks 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: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: Young-Seok Kim <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: No
