asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Blow (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: [ASTERIXDB-2204][STO] Fix implementations and usages of IInd...
Date Sun, 11 Feb 2018 20:29:58 GMT
Michael Blow has posted comments on this change.

Change subject: [ASTERIXDB-2204][STO] Fix implementations and usages of IIndexCursor
......................................................................


Patch Set 20:

(4 comments)

https://asterix-gerrit.ics.uci.edu/#/c/2324/20/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/DestroyUtils.java
File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/util/DestroyUtils.java:

PS20, Line 42: public static Throwable destroy(IDestroyable destroyable, Throwable root)
can we refactor callers of this to pass the throwable first, then just inline this method
at line 36 above?  it seems strange to have both destroy(IDestroyable destroyable, Throwable
root) and destroy(Throwable root, IDestroyable destroyable) be valid signatures.


https://asterix-gerrit.ics.uci.edu/#/c/2324/20/hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java
File hyracks-fullstack/hyracks/hyracks-storage-am-lsm-btree/src/main/java/org/apache/hyracks/storage/am/lsm/btree/impls/LSMBTreeOpContext.java:

PS20, Line 189: null, 
can remove this now, right?


PS20, Line 190:         failure = DestroyUtils.destroy(failure, mutableBTreeOpCtxs);
              :         failure = DestroyUtils.destroy(insertSearchCursor, failure);
see comment in DestroyUtils, very confusing to support both parameter orders for DestroyUtils.destroy()


PS20, Line 189:        Throwable failure = DestroyUtils.destroy(null, mutableBTreeAccessors);
              :         failure = DestroyUtils.destroy(failure, mutableBTreeOpCtxs);
              :         failure = DestroyUtils.destroy(insertSearchCursor, failure);
              :         failure = DestroyUtils.destroy(memCursor, failure);
can't we just have one statement here:

Throwable failure = DestroyUtils.destroy(mutableBTreeAccessors, mutableBTreeOpCtxs, insertSearchCursor,
memCursor)?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I98a7a8b931eb24dbe11bf2bdc61b754ca28ebdf9
Gerrit-PatchSet: 20
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Luo Chen <cluo8@uci.edu>
Gerrit-Reviewer: Michael Blow <mblow@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mhubail@apache.org>
Gerrit-Reviewer: Taewoo Kim <taewok2@uci.edu>
Gerrit-Reviewer: Taewoo Kim <wangsaeu@gmail.com>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamousaa@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message