Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9089 )

Change subject: IMPALA-6215: Removes race when using LibCache.
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

> (2 comments)
 >
 > Absolutely agree that lib-cache api is bit clunky. Main thing with
 > this patch is the fix so that we can run the tests as before (I've
 > reverted my previous change in this patch). I've kept the api the
 > same as GetSoFunctionPtr for uniformity but agree that this should
 > be simplified. I'll file a JIRA-- what's your favorite current
 > handle api in the code base for reference?

Sounds good. I cant think of a handle API off the top of my head that would be 
suitable for this, but please go ahead and file the JIRA.

http://gerrit.cloudera.org:8080/#/c/9089/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

http://gerrit.cloudera.org:8080/#/c/9089/2/be/src/codegen/llvm-codegen.cc@333
PS2, Line 333:  MakeScopeExitTrigger([entry]() {
             :     if (entry != nullptr) 
LibCache::instance()->DecrementUseCount(entry);}
             :   );
> Isn't LibCacheEntry shared between all of the callers?
oh yea thats right.


http://gerrit.cloudera.org:8080/#/c/9089/3/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/9089/3/tests/query_test/test_udfs.py@440
PS3, Line 440:
nit: extra space at the end, look for red boxes



--
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: 3
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: Wed, 24 Jan 2018 19:41:34 +0000
Gerrit-HasComments: Yes

Reply via email to