You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org> on 2018/04/10 15:58:29 UTC

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Vuk Ercegovac has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9968


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

IMPALA-6215: Removes race when using LibCache.

Re-do the previously reverted change for IMPALA-6215. This patch
addresses the flakes listed in IMPALA-6092, which have become more
urgent recently.

LibCache's api to provide access to locally cached files has a race.
Currently, the client of the cache accesses the locally cached path
as a string, but nothing guarantees that the associated file is not
removed before the client is done using it. This race is suspected
as the root cause for the flakiness seen in IMPALA-6092. These tests
fail once in a while with classloader errors unable to load java udf
classes. In these tests, the lib cache makes no guarantee that the path
to the jar will remain valid from the time the path is acquired through
the time needed to fetch the jar and resolve the needed classes.

LibCache offers liveness guarantees for shared objects via reference
counting. The fix in this patch extends this API to also cover paths
to locally cached files.

This fix *only* addresses the path race. General cleanup of the api will
be done separately.

Testing:
   - added a test to test_udfs.py that does many concurrent udf uses and
     removals. By increasing the concurrent operations to 100, the issue
     in IMPALA-6092 is locally reproducible on every run. With this fix,
     the problem is no longer reproducible with this test.

Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
---
M be/src/codegen/llvm-codegen.cc
M be/src/exec/external-data-source-executor.cc
M be/src/exprs/hive-udf-call.cc
M be/src/exprs/hive-udf-call.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M tests/query_test/test_udfs.py
8 files changed, 163 insertions(+), 50 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/9968/1
-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9968 )

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


Patch Set 1: Code-Review+2

(1 comment)

carrying Tim's +2

http://gerrit.cloudera.org:8080/#/c/9968/1/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/9968/1/tests/query_test/test_udfs.py@548
PS1, Line 548:       print "use create: " + s
> leftover debugging print?
whoops, removed.



-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 20:51:45 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9968 )

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


Patch Set 2:

noticed that the test litters the test cluster with jars. latest patch makes a small change to put the temp jar in the test db so that it gets cleaned up. 
verified this change by re-running test_udfs.


-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 21:34:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9968 )

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


Patch Set 2: Code-Review+2

carrying tim's +2


-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 20:52:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9968 )

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


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 02:14:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9968 )

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

IMPALA-6215: Removes race when using LibCache.

Re-do the previously reverted change for IMPALA-6215. This patch
addresses the flakes listed in IMPALA-6092, which have become more
urgent recently.

LibCache's api to provide access to locally cached files has a race.
Currently, the client of the cache accesses the locally cached path
as a string, but nothing guarantees that the associated file is not
removed before the client is done using it. This race is suspected
as the root cause for the flakiness seen in IMPALA-6092. These tests
fail once in a while with classloader errors unable to load java udf
classes. In these tests, the lib cache makes no guarantee that the path
to the jar will remain valid from the time the path is acquired through
the time needed to fetch the jar and resolve the needed classes.

LibCache offers liveness guarantees for shared objects via reference
counting. The fix in this patch extends this API to also cover paths
to locally cached files.

This fix *only* addresses the path race. General cleanup of the api will
be done separately.

Testing:
   - added a test to test_udfs.py that does many concurrent udf uses and
     removals. By increasing the concurrent operations to 100, the issue
     in IMPALA-6092 is locally reproducible on every run. With this fix,
     the problem is no longer reproducible with this test.

Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Reviewed-on: http://gerrit.cloudera.org:8080/9968
Reviewed-by: Vuk Ercegovac <ve...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/codegen/llvm-codegen.cc
M be/src/exec/external-data-source-executor.cc
M be/src/exprs/hive-udf-call.cc
M be/src/exprs/hive-udf-call.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M tests/query_test/test_udfs.py
8 files changed, 162 insertions(+), 50 deletions(-)

Approvals:
  Vuk Ercegovac: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/9968

to look at the new patch set (#3).

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

IMPALA-6215: Removes race when using LibCache.

Re-do the previously reverted change for IMPALA-6215. This patch
addresses the flakes listed in IMPALA-6092, which have become more
urgent recently.

LibCache's api to provide access to locally cached files has a race.
Currently, the client of the cache accesses the locally cached path
as a string, but nothing guarantees that the associated file is not
removed before the client is done using it. This race is suspected
as the root cause for the flakiness seen in IMPALA-6092. These tests
fail once in a while with classloader errors unable to load java udf
classes. In these tests, the lib cache makes no guarantee that the path
to the jar will remain valid from the time the path is acquired through
the time needed to fetch the jar and resolve the needed classes.

LibCache offers liveness guarantees for shared objects via reference
counting. The fix in this patch extends this API to also cover paths
to locally cached files.

This fix *only* addresses the path race. General cleanup of the api will
be done separately.

Testing:
   - added a test to test_udfs.py that does many concurrent udf uses and
     removals. By increasing the concurrent operations to 100, the issue
     in IMPALA-6092 is locally reproducible on every run. With this fix,
     the problem is no longer reproducible with this test.

Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
---
M be/src/codegen/llvm-codegen.cc
M be/src/exec/external-data-source-executor.cc
M be/src/exprs/hive-udf-call.cc
M be/src/exprs/hive-udf-call.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M tests/query_test/test_udfs.py
8 files changed, 162 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/9968/3
-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9968 )

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


Patch Set 3: Code-Review+2

carrying tim's +2


-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 21:50:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9968 )

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


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2275/


-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 3
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 22:08:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9968 )

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


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9968/1/tests/query_test/test_udfs.py
File tests/query_test/test_udfs.py:

http://gerrit.cloudera.org:8080/#/c/9968/1/tests/query_test/test_udfs.py@548
PS1, Line 548:       print "use create: " + s
leftover debugging print?



-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 1
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Apr 2018 18:52:57 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6215: Removes race when using LibCache.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/9968

to look at the new patch set (#2).

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

IMPALA-6215: Removes race when using LibCache.

Re-do the previously reverted change for IMPALA-6215. This patch
addresses the flakes listed in IMPALA-6092, which have become more
urgent recently.

LibCache's api to provide access to locally cached files has a race.
Currently, the client of the cache accesses the locally cached path
as a string, but nothing guarantees that the associated file is not
removed before the client is done using it. This race is suspected
as the root cause for the flakiness seen in IMPALA-6092. These tests
fail once in a while with classloader errors unable to load java udf
classes. In these tests, the lib cache makes no guarantee that the path
to the jar will remain valid from the time the path is acquired through
the time needed to fetch the jar and resolve the needed classes.

LibCache offers liveness guarantees for shared objects via reference
counting. The fix in this patch extends this API to also cover paths
to locally cached files.

This fix *only* addresses the path race. General cleanup of the api will
be done separately.

Testing:
   - added a test to test_udfs.py that does many concurrent udf uses and
     removals. By increasing the concurrent operations to 100, the issue
     in IMPALA-6092 is locally reproducible on every run. With this fix,
     the problem is no longer reproducible with this test.

Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
---
M be/src/codegen/llvm-codegen.cc
M be/src/exec/external-data-source-executor.cc
M be/src/exprs/hive-udf-call.cc
M be/src/exprs/hive-udf-call.h
M be/src/runtime/lib-cache.cc
M be/src/runtime/lib-cache.h
M be/src/service/fe-support.cc
M tests/query_test/test_udfs.py
8 files changed, 162 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/9968/2
-- 
To view, visit http://gerrit.cloudera.org:8080/9968
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I72ac0dfb13cf37d79e25c5b8a258b65f2dad097f
Gerrit-Change-Number: 9968
Gerrit-PatchSet: 2
Gerrit-Owner: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>