Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9089 )
Change subject: IMPALA-6215: Removes race when using LibCache. ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/9089/6/be/src/runtime/lib-cache.h File be/src/runtime/lib-cache.h: http://gerrit.cloudera.org:8080/#/c/9089/6/be/src/runtime/lib-cache.h@49 PS6, Line 49: When the caller requests a ptr into the library, they : /// are given the entry handle and must decrement the ref count when they : /// are done. > maybe clarify this to indicate that a handle's scope takes care of that? updated to reflect the current state http://gerrit.cloudera.org:8080/#/c/9089/6/be/src/runtime/lib-cache.h@79 PS6, Line 79: /// '*entry' is expected to be null. The caller must call DecrementUseCount(*entry) : /// when it is done using 'path'. > that needs updating. Done http://gerrit.cloudera.org:8080/#/c/9089/6/be/src/runtime/lib-cache.h@185 PS6, Line 185: set_entry > since not strictly a "setter", probably should be SetEntry(). Done http://gerrit.cloudera.org:8080/#/c/9089/6/be/src/runtime/lib-cache.cc File be/src/runtime/lib-cache.cc: http://gerrit.cloudera.org:8080/#/c/9089/6/be/src/runtime/lib-cache.cc@194 PS6, Line 194: ( > remove extraneous parens Done http://gerrit.cloudera.org:8080/#/c/9089/6/be/src/runtime/lib-cache.cc@196 PS6, Line 196: ( > same Done -- To view, visit http://gerrit.cloudera.org:8080/9089 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9175085201fe8b11424ab8f88d7b992cb7b0daea Gerrit-Change-Number: 9089 Gerrit-PatchSet: 6 Gerrit-Owner: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Mon, 29 Jan 2018 23:35:00 +0000 Gerrit-HasComments: Yes
