Luo Chen has posted comments on this change. Change subject: Add LSMDiskComponentBulkLoader ......................................................................
Patch Set 5: > I'm not sure I follow. It seems that the methods do the exact same > thing. A consumer of the interface adds tuples to something and > then either marks a successful completion (end) or an error case > (abort). The differences that you point out seem to be in the > implementation, while the contract of the interface seems to be the > same. > Following your argument it seems that we should have a new > "Comparable" interface for every type that we want to compare even > tough the interface provides a contract for the compareTo method > only. > And I would also think that it’s not a bad sign, if an index > component implements the same interface as the index that it is a > part of. I guess that there's not such a big difference between a > single-component index and an index component. I think now we agree on that adding the implementation of LSMDiskComponentBulkLoader is fine (since it simplifies/unifies operations like flush/merge/index bulk load), but the question is that whether we need a new interface ILSMDiskComponentBulkLoader. Without this interface, the implementations should implement IIndexBulkLoader. For this question, the main point is that whether a disk component should be treated as an index. Personally, I think the answer is no, and the current interface hierarchy of LSMIndex/Index is also separate from LSMComponent. Making LSMComponentBulkLoader implement IIndexBulkLoader would make the code a little bit confusing. Ideally, I think there should be an interface called IBulkLoader, which is used to bulk load data into some target. IIndexBulkLoader and ILSMDiskComponentBulkLoader should be two sub interfaces of this IBulkLoader sitting in parallel. Hope this explains the motivation of this patch. -- To view, visit https://asterix-gerrit.ics.uci.edu/1773 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I772a6d68761fcbb85982a1c9f72f2d186e1d1ffb Gerrit-PatchSet: 5 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Luo Chen <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Jianfeng Jia <[email protected]> Gerrit-Reviewer: Luo Chen <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: No
