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 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/9089/4/be/src/runtime/lib-cache.h File be/src/runtime/lib-cache.h: http://gerrit.cloudera.org:8080/#/c/9089/4/be/src/runtime/lib-cache.h@78 PS4, Line 78: entry is non-null > I don't see any callers where entry is not provided - can we remove? Would yup, removed. I first went for consistency with the other one (GetSoFunctionPtr) but since the plan is to change both of these, I went ahead and simplified this one. http://gerrit.cloudera.org:8080/#/c/9089/4/be/src/service/fe-support.cc File be/src/service/fe-support.cc: http://gerrit.cloudera.org:8080/#/c/9089/4/be/src/service/fe-support.cc@299 PS4, Line 299: Status status = LibCache::instance()->GetLocalPath( > Should this use the ScopeExitTrigger pattern? I guess it's not necessary bu right, I left it off since the current code did not need it. but that's a fair point about consistency and will help if anyone changes this area in the future, so I've added it. http://gerrit.cloudera.org:8080/#/c/9089/4/be/src/service/fe-support.cc@400 PS4, Line 400: LibCache::instance()->DecrementUseCount(entry); > Same question here. didn't need it, but added it for consistency. -- 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: 4 Gerrit-Owner: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Mon, 29 Jan 2018 21:39:39 +0000 Gerrit-HasComments: Yes
