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 2: (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? 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? yes, it's shared between callers (same deal with the GetSoFunctionPtr) http://gerrit.cloudera.org:8080/#/c/9089/2/tests/query_test/test_udfs.py File tests/query_test/test_udfs.py: http://gerrit.cloudera.org:8080/#/c/9089/2/tests/query_test/test_udfs.py@443 PS2, Line 443: create_fn_to_drop = "create function {0}.foo_{1}() returns string LOCATION '{2}' SYMBOL='org.apache.impala.TestUpdateUdf'" : create_fn_to_use = "create function {0}.use_it(string) returns string LOCATION '{1}' SYMBOL='org.apache.impala.TestUdf'" : drop_fn = "drop function if exists {0}.foo_{1}()" : use_fn = "select * from (select max(int_col) from functional.alltypesagg where {0}.use_it(string_col) = 'blah' union all (select max(int_col) from functional.alltypesagg where {0}.use_it(String_col) > '1' union all (select max(int_col) from functional.alltypesagg where {0}.use_it(string_col) > '1'))) v" : > nit: long lines 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: 2 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 02:56:17 +0000 Gerrit-HasComments: Yes
