[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Michael Ho has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8597 Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" Compilation failed on some platforms as ‘EVP_aes_256_ctr’ not declared in openssl-util.cc. Reverting the change to unbreak the builds for now. This reverts commit fb4c3b01240d8f65fc2c45bf27b668ae9b1fa5d2. Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb --- M be/src/runtime/tmp-file-mgr.cc M be/src/util/openssl-util.cc M be/src/util/openssl-util.h 3 files changed, 11 insertions(+), 22 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/97/8597/1 -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho
[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Xianda Ke has posted comments on this change. ( http://gerrit.cloudera.org:8080/8597 ) Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. Patch Set 1: Code-Review+1 > Assignee added: Sailesh Mukil it is ok to revert since it blocks compiling now. I'll investigate it to find out a solution for CTR mode. -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Xianda Ke Gerrit-Comment-Date: Mon, 20 Nov 2017 09:23:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8490 ) Change subject: IMPALA-2248: Make idle_session_timeout a query option .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/8490/6/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/8490/6/be/src/service/client-request-state.cc@210 PS6, Line 210: &session_->set_query_options_mask)); > By unset I mean "set blah ''", i.e. setting the query option to the empty s Thanks for your comment, it revealed a couple of bugs in my commit. Unsetting: Oh I see, then the answer is yes, we can unset the session timeout, after that it will be equal to the process-wide option FLAGS_idle_session_timeout. Setting query options: The confOverlay that is sent with a query only has effect on that query, and I think setting session timeout doesn't make sense for an individual query, so in this commit I ignore that case. AFAIK, from a given session we can send queries to different request pools. If this is the case, I think the pool configuration should not modify idle_session_timeout. Pool configurations are more about queries rather than sessions if I understood correctly. Pytests: I extended the pytests. I also kept a test case in JdbcTest.java, because JDBC clients are the main target of this commit. Query Option layering: I added a comment to SetTimeout that it changes the query options. It modifies set_query_options (the query options that are set explicitly). It was simple to do that, and the result of SET shows the right value. On the other hand, it does break the layering of this option. I can also keep the layering, but then I would need to inject extra if-else statements to a few more places, and currently I don't see that it would worth it. -- To view, visit http://gerrit.cloudera.org:8080/8490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e Gerrit-Change-Number: 8490 Gerrit-PatchSet: 7 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 20 Nov 2017 15:05:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option
Hello Michael Ho, Thomas Tauber-Marshall, Laszlo Gaal, Gabor Kaszab, Attila Jeges, Tim Armstrong, Csaba Ringhofer, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8490 to look at the new patch set (#8). Change subject: IMPALA-2248: Make idle_session_timeout a query option .. IMPALA-2248: Make idle_session_timeout a query option This commit makes idle_session_timeout a query option. idle_session_timeout currently can be set as a command line option, which will be the default timeout for sessions. HS2 sessions can override it with a smaller value by setting it in the configuration overlay of HS2 OpenSession(). However, we can't override idle_session_timeout for JDBC/ODBC connections, because we cannot put this in the connection string. This commit is a workaround for this problem, it allows JDBC/ODBC connections to set the session timeout as a query option with the SET statement. I created an automated test case in JdbcTest.java based on test_hs2.py::test_concurrent_session_mixed_idle_timeout. I also extended the test_session_expiration and test_set_and_unset test suites. Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/util/Metrics.java M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_set_and_unset.py 13 files changed, 339 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/8 -- To view, visit http://gerrit.cloudera.org:8080/8490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e Gerrit-Change-Number: 8490 Gerrit-PatchSet: 8 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-2248: Make idle session timeout a query option
Hello Michael Ho, Thomas Tauber-Marshall, Laszlo Gaal, Gabor Kaszab, Attila Jeges, Tim Armstrong, Csaba Ringhofer, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8490 to look at the new patch set (#9). Change subject: IMPALA-2248: Make idle_session_timeout a query option .. IMPALA-2248: Make idle_session_timeout a query option This commit makes idle_session_timeout a query option. idle_session_timeout currently can be set as a command line option, which will be the default timeout for sessions. HS2 sessions can override it with a smaller value by setting it in the configuration overlay of HS2 OpenSession(). However, we can't override idle_session_timeout for JDBC/ODBC connections, because we cannot put this in the connection string. This commit is a workaround for this problem, it allows JDBC/ODBC connections to set the session timeout as a query option with the SET statement. I created an automated test case in JdbcTest.java based on test_hs2.py::test_concurrent_session_mixed_idle_timeout. I also extended the test_session_expiration and test_set_and_unset test suites. Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift M fe/src/test/java/org/apache/impala/service/JdbcTest.java A fe/src/test/java/org/apache/impala/util/Metrics.java M tests/custom_cluster/test_session_expiration.py M tests/custom_cluster/test_set_and_unset.py 13 files changed, 340 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/8490/9 -- To view, visit http://gerrit.cloudera.org:8080/8490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e Gerrit-Change-Number: 8490 Gerrit-PatchSet: 9 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8597 ) Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xianda Ke Gerrit-Comment-Date: Mon, 20 Nov 2017 17:00:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6217: fix DCHECK in Parquet fuzz test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8594 ) Change subject: IMPALA-6217: fix DCHECK in Parquet fuzz test .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1501/ -- To view, visit http://gerrit.cloudera.org:8080/8594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ceca7de31f602b75d744dacbdf37afa75983344 Gerrit-Change-Number: 8594 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Mon, 20 Nov 2017 17:04:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8510 ) Change subject: IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/8510/7/be/src/util/openssl-util.cc File be/src/util/openssl-util.cc: http://gerrit.cloudera.org:8080/#/c/8510/7/be/src/util/openssl-util.cc@147 PS7, Line 147: return (SSLeay() >= OPENSSL_VERSION_1_0_1); When we re-do this patch we should also add a test option to force use of CFB mode and make sure that the unit test tests both encryption modes. -- To view, visit http://gerrit.cloudera.org:8080/8510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib97939f2334838263364b53ef3413871638bf53e Gerrit-Change-Number: 8510 Gerrit-PatchSet: 7 Gerrit-Owner: Xianda Ke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Mike Yoder Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xianda Ke Gerrit-Comment-Date: Mon, 20 Nov 2017 17:57:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2181: Add query option levels for display
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/8447 ) Change subject: IMPALA-2181: Add query option levels for display .. Patch Set 14: FYI, both the core and the exhaustive tests passed for this patch. -- To view, visit http://gerrit.cloudera.org:8080/8447 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75720d0d454527e1a0ed19bb43cf9e4f018ce1d1 Gerrit-Change-Number: 8447 Gerrit-PatchSet: 14 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 20 Nov 2017 18:23:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8597 ) Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1502/ -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xianda Ke Gerrit-Comment-Date: Mon, 20 Nov 2017 18:25:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6092: avoid drop/create function interactions in e2e tests
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8593 ) Change subject: IMPALA-6092: avoid drop/create function interactions in e2e tests .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG@10 PS1, Line 10: IMPALA-6215 explains a race between the lib_cache > Absolutely we should fix 6215, but we should also avoid unspecified test in IMO, this is not an "inadvertent interaction". I'm pretty sure many users use the UDFs this way (have a single source fat jar and reference it across multiple functions/queries/DDLs). My feeling is that the way this e-e test case is written, it provides test coverage for that use case scenario and I feel we shouldn't change that. I'm not too strong about this, but may be better to check with Dimitris/Alex too. -- To view, visit http://gerrit.cloudera.org:8080/8593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica3538788b1d2ab5e361261e2ade62780b838e65 Gerrit-Change-Number: 8593 Gerrit-PatchSet: 1 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Nov 2017 18:36:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1144: Fix exception when cancelling query in Impala-shell with CTRL-C
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8549 ) Change subject: IMPALA-1144: Fix exception when cancelling query in Impala-shell with CTRL-C .. Patch Set 5: (10 comments) Overall approach seems good to me, but had quite a few comments about comments and naming. http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG@25 PS2, Line 25: Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 > Thanks for the nice example, Phil! It seems ok that it doesn't reproduce it every time - it's inherently racy. We could probably have a deterministic unit test by mocking out enough things, but I think I prefer the end-to-end shell test. It would be good to also test on release and ASAN builds to make sure that it doesn't immediately fail when the daemon is slow or fast. http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@58 PS5, Line 58: QueryCancelledException Can we call this QueryCancelledByShellException or ExpectedQueryCancellationException or something like that to distinguish from the case when the query was cancelled unexpectedly (e.g. by an admin from the impala debug ui). http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@60 PS5, Line 60: self.value = value The value might not be necessary if we're always going to pass in the same string. The existing exceptions do this but seems like an odd pattern. We could just print that string instead of stringifying the exception. http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@83 PS5, Line 83: self.is_query_cancelled = False Can you add a brief comment explaining how this works. I.e. that it's set to true by the cancellation signal handler and suppresses any errors after cancellation. http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@435 PS5, Line 435: except BeeswaxService.QueryNotFoundException: It's weird that we don't need the logic for QueryNotFoundException. Does Impala even throw this? I'm ok with leaving this in since it's part of the beeswax protocol but we should also check for cancellation for consistency. http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_shell.py@1026 PS5, Line 1026: print_to_stderr(str(e)) It might be to just not print anything - we don't always throw this exception when the query is cancelled so the message only gets printed sometimes. http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@333 PS5, Line 333: 'select * from v, v v2, v v3, v v4, v v5, v v6, v v7, v v8, v v9;"' Maybe add a few more tables to the cross join to make sure that it runs for a very long time. We don't want a flaky test if it sometimes finishes quicker. http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@342 PS5, Line 342:'tpch.customer c3 order by c1.c_name"' You could also do something like: order by c1.c_name, sleep(1) That would make it pause 1ms per row and slow it down further. http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@346 PS5, Line 346: query_txt Isn't this the command-line arguments? http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@349 PS5, Line 349: sleep(3) Can you add a comment to the function explaining that it waits 3 seconds before cancelling and expects the query to be cancelled. Is there any reason why 3 seconds is the right amount of time? Currently it just seems like a magic number. Would be good to reduce it if possible, since it will make the tests very slow (other shell tests have this problem already). -- To view, visit http://gerrit.cloudera.org:8080/8549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 Gerrit-Change-Number: 8549 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 20 Nov 2017 18:41:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Hello Lars Volker, Philip Zeyliger, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8546 to look at the new patch set (#6). Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo Running shell commands from impalad can be problematic, because using popen() leads to forking which causes a spike in virtual memory. To avoid this, "ls" is replaced with POSIX API calls. FileDescriptorMap fd_desc_ was only used to get the number of file descriptors, so it was unneccesery work to initialize it. It is removed, and only the number of file descriptors is computed. The automatic test for this function is only a sanity check, because there is no way to know the "expected value" in advance, and the number of file desciptors can change anytime. Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 --- M be/src/util/proc-info-test.cc M be/src/util/process-state-info.cc M be/src/util/process-state-info.h 3 files changed, 35 insertions(+), 44 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/8546/6 -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/8546/5/be/src/util/process-state-info.h File be/src/util/process-state-info.h: http://gerrit.cloudera.org:8080/#/c/8546/5/be/src/util/process-state-info.h@61 PS5, Line 61: /// Below are some of the Process status information that will be read > nit: long line. Please have a look at how to configure your text editor to Done http://gerrit.cloudera.org:8080/#/c/8546/5/be/src/util/process-state-info.cc File be/src/util/process-state-info.cc: http://gerrit.cloudera.org:8080/#/c/8546/5/be/src/util/process-state-info.cc@36 PS5, Line 36: using boost::algorithm::is_any_of; > nit: sort alphabetically? Done http://gerrit.cloudera.org:8080/#/c/8546/5/be/src/util/process-state-info.cc@157 PS5, Line 157: // readdir() is not thread-safe according to its man page, but in glibc > Can you include a reference to the source (e.g. https://www.gnu.org/softwar Done http://gerrit.cloudera.org:8080/#/c/8546/6/be/src/util/process-state-info.cc File be/src/util/process-state-info.cc: http://gerrit.cloudera.org:8080/#/c/8546/6/be/src/util/process-state-info.cc@161 PS6, Line 161: if(dir_entry->d_name[0] != '.') ++fd_count; // . and .. do not count www.gnu.org/software/libc/manual/html_node/Reading_002fClosing-Directory.html states: "Portability Note: On some systems readdir may not return entries for . and ..," Because of this, the filename must be checked to return the exact number of descriptors. As every filename in fd directory is a number, it is enough to check the first character. -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 20 Nov 2017 18:51:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6217: fix DCHECK in Parquet fuzz test
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8594 ) Change subject: IMPALA-6217: fix DCHECK in Parquet fuzz test .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ceca7de31f602b75d744dacbdf37afa75983344 Gerrit-Change-Number: 8594 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Mon, 20 Nov 2017 18:55:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6092: avoid drop/create function interactions in e2e tests
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8593 ) Change subject: IMPALA-6092: avoid drop/create function interactions in e2e tests .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG@10 PS1, Line 10: IMPALA-6215 explains a race between the lib_cache > IMO, this is not an "inadvertent interaction". I'm pretty sure many users u The test that's changed was testing for an invalid symbol that was causing a backend crash when cleaning up... the interaction with some other test, while valid, is inadvertent (and difficult to diagnose). My preference is to reign in the current test with this change so that its really testing what it intends to test and to make an explicit test for the buggy interaction with 6215. Looks like we need more coverage for this type of interaction, since I agree that its likely common. -- To view, visit http://gerrit.cloudera.org:8080/8593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica3538788b1d2ab5e361261e2ade62780b838e65 Gerrit-Change-Number: 8593 Gerrit-PatchSet: 1 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Nov 2017 19:00:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6210: Add query id to lineage graph logging
Tianyi Wang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/8589 ) Change subject: IMPALA-6210: Add query id to lineage graph logging .. IMPALA-6210: Add query id to lineage graph logging Some tools use lineage graph logging to collect query metrics. Currently only query hash is present in this log. Adding query id into it makes such accounting easier. Testing: The equality of query id in the query profile and lineage log is checked in test_lineage.py. A test for TUniqueIdUtil is added to the FE tests. Change-Id: I4adbd02df37a234dbb79f58b7c46ca11a914229f --- M be/src/util/lineage-util.h M common/thrift/LineageGraph.thrift M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java A fe/src/main/java/org/apache/impala/util/TUniqueIdUtil.java A fe/src/test/java/org/apache/impala/util/TUniqueIdUtilTest.java M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test M tests/custom_cluster/test_lineage.py 7 files changed, 185 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/8589/2 -- To view, visit http://gerrit.cloudera.org:8080/8589 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4adbd02df37a234dbb79f58b7c46ca11a914229f Gerrit-Change-Number: 8589 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-6092: avoid drop/create function interactions in e2e tests
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8593 ) Change subject: IMPALA-6092: avoid drop/create function interactions in e2e tests .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG@10 PS1, Line 10: IMPALA-6215 explains a race between the lib_cache > The test that's changed was testing for an invalid symbol that was causing Fair point, I agree that we caught the bug by fluke when the test case was doing something totally irrelevant. Like we discussed, I'm ok as long as we fix the test coverage with proper tests that rely on a single source jar and multiple DDLs (either in this change or with IMPALA-6215) http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG@13 PS1, Line 13: test_udf_invalid_symbol drops a function from that jar, which causes the use nit: line overflow in multiple places. http://gerrit.cloudera.org:8080/#/c/8593/1/tests/query_test/test_udfs.py File tests/query_test/test_udfs.py: http://gerrit.cloudera.org:8080/#/c/8593/1/tests/query_test/test_udfs.py@422 PS1, Line 422: check_call(["hadoop", "fs", "-put", "-f", src_udf_path, tgt_udf_path]) Just to be sure, this works ok on local fs right? Basically we "put" from local fs to local fs. -- To view, visit http://gerrit.cloudera.org:8080/8593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica3538788b1d2ab5e361261e2ade62780b838e65 Gerrit-Change-Number: 8593 Gerrit-PatchSet: 1 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Nov 2017 19:40:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8597 ) Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. Patch Set 1: @kwho I'm ok with merging this as-is, given that it compiled ok and it's a clean revert. Feel free to +1 verify and submit it if you like. -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xianda Ke Gerrit-Comment-Date: Mon, 20 Nov 2017 19:58:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6092: avoid drop/create function interactions in e2e tests
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8593 ) Change subject: IMPALA-6092: avoid drop/create function interactions in e2e tests .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8593/1//COMMIT_MSG@13 PS1, Line 13: test_udf_invalid_symbol drops a function from that jar, which causes the use > nit: line overflow in multiple places. Done http://gerrit.cloudera.org:8080/#/c/8593/1/tests/query_test/test_udfs.py File tests/query_test/test_udfs.py: http://gerrit.cloudera.org:8080/#/c/8593/1/tests/query_test/test_udfs.py@422 PS1, Line 422: check_call(["hadoop", "fs", "-put", "-f", src_udf_path, tgt_udf_path]) > Just to be sure, this works ok on local fs right? Basically we "put" from l Couple of points. This always copies from a local source and it renames to a unique tgt name in local fs. Its also the way that the create/drop tests are also setup (they're careful not to use the same shared jar across tests). I assume the other tests work, but just in case-- how do I run just the local fs version of the test to verify this? -- To view, visit http://gerrit.cloudera.org:8080/8593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica3538788b1d2ab5e361261e2ade62780b838e65 Gerrit-Change-Number: 8593 Gerrit-PatchSet: 1 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Nov 2017 20:22:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6092: avoid drop/create function interactions in e2e tests
Hello Bharath Vissapragada, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8593 to look at the new patch set (#2). Change subject: IMPALA-6092: avoid drop/create function interactions in e2e tests .. IMPALA-6092: avoid drop/create function interactions in e2e tests The e2e unit tests for udfs can interact via the backend lib_cache, causing test flakes. IMPALA-6215 explains a race between the lib_cache and UdfExecutor in the frontend which is the likely the root cause. Two e2e tests use the same jar (test_java_udfs and test_udf_invalid_symbol), test_udf_invalid_symbol drops a function from that jar, which causes the use of that jar to fail in the test_java_udfs test. Since the state of lib_cache is per process, its state causes these interactions across unit tests. This change avoids the interactions by using separate jars for the separate tests. Change-Id: Ica3538788b1d2ab5e361261e2ade62780b838e65 --- M be/src/runtime/lib-cache.h M fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java M tests/query_test/test_udfs.py 3 files changed, 12 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/8593/2 -- To view, visit http://gerrit.cloudera.org:8080/8593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ica3538788b1d2ab5e361261e2ade62780b838e65 Gerrit-Change-Number: 8593 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6217: fix DCHECK in Parquet fuzz test
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8594 ) Change subject: IMPALA-6217: fix DCHECK in Parquet fuzz test .. IMPALA-6217: fix DCHECK in Parquet fuzz test The IMPALA-4177 change accidentally removed a Status check that could be hit with a corrupt parquet file. Testing: Ran TestScannersFuzzing in a loop for 2 days. Change-Id: I6ceca7de31f602b75d744dacbdf37afa75983344 Reviewed-on: http://gerrit.cloudera.org:8080/8594 Reviewed-by: Alex Behm Tested-by: Impala Public Jenkins --- M be/src/exec/parquet-column-readers.cc 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: Alex Behm: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6ceca7de31f602b75d744dacbdf37afa75983344 Gerrit-Change-Number: 8594 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] IMPALA-6217: fix DCHECK in Parquet fuzz test
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8594 ) Change subject: IMPALA-6217: fix DCHECK in Parquet fuzz test .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8594 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6ceca7de31f602b75d744dacbdf37afa75983344 Gerrit-Change-Number: 8594 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Mon, 20 Nov 2017 20:37:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/8309 ) Change subject: IMPALA-5019: Decimal V2 addition .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc@a2402 PS4, Line 2402: > Still not following. The old V2 expected value was 999.6 not 999.5. Regarding the first point: the type of "power(10, 3) - 0.45" in decimal v2 before this patch is decimal(38, 17). the type of "power(10, 3) - 0.45" in decimal v2 after this patch is decimal(38, 16). This causes the result of the conversion to double to be slightly different. Before this patch (in decimal v2) after converting to double it looks like this: 999.5500156861889 After this patch, it looks like this: 999.5499727558438 This is why the result was 999.5 and not 999.6 after the patch. This is all due to IMPALA-6183, which was fixed (and is reflected in the latest patch). -- To view, visit http://gerrit.cloudera.org:8080/8309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336 Gerrit-Change-Number: 8309 Gerrit-PatchSet: 4 Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Mon, 20 Nov 2017 21:34:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Michael Ho has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8597 ) Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" Compilation failed on some platforms as ‘EVP_aes_256_ctr’ not declared in openssl-util.cc. Reverting the change to unbreak the builds for now. This reverts commit fb4c3b01240d8f65fc2c45bf27b668ae9b1fa5d2. Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Reviewed-on: http://gerrit.cloudera.org:8080/8597 Reviewed-by: Xianda Ke Reviewed-by: Tim Armstrong Tested-by: Michael Ho --- M be/src/runtime/tmp-file-mgr.cc M be/src/util/openssl-util.cc M be/src/util/openssl-util.h 3 files changed, 11 insertions(+), 22 deletions(-) Approvals: Xianda Ke: Looks good to me, but someone else must approve Tim Armstrong: Looks good to me, approved Michael Ho: Verified -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xianda Ke
[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8597 ) Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. Patch Set 1: I also did private build run last night. Let me just merge it without waiting for GVO. -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xianda Ke Gerrit-Comment-Date: Mon, 20 Nov 2017 21:50:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8597 ) Change subject: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow" .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8597 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb Gerrit-Change-Number: 8597 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Xianda Ke Gerrit-Comment-Date: Mon, 20 Nov 2017 21:50:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 20 Nov 2017 21:56:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 20 Nov 2017 21:56:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1503/ -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 20 Nov 2017 21:57:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8309 ) Change subject: IMPALA-5019: Decimal V2 addition .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc@a2402 PS4, Line 2402: > Regarding the first point: I'm not sure what the intent of the original test case is and why it was written this way. Zach added this test case as part of the rounding for cast to DECIMAL, so it seems we may be losing some coverage he intended to get. Could you check with Zach to see what he thinks and if we should rewrite this (and other) test cases? If we leave this test case here, I think a comment explaining why 999.5 is the "correct" answer would be helpful, given that CAST(999.55 as DECIMAL(4.1)) should normally result in 999.6 -- To view, visit http://gerrit.cloudera.org:8080/8309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336 Gerrit-Change-Number: 8309 Gerrit-PatchSet: 4 Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Mon, 20 Nov 2017 22:01:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR](asf-site) Make wording around adopters of Impala more consistent.
Lars Volker has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8609 Change subject: Make wording around adopters of Impala more consistent. .. Make wording around adopters of Impala more consistent. Change-Id: I728f86fbf2db3d201c53c8ea6182727006e9c7fc --- M index.html 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/8609/1 -- To view, visit http://gerrit.cloudera.org:8080/8609 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: asf-site Gerrit-MessageType: newchange Gerrit-Change-Id: I728f86fbf2db3d201c53c8ea6182727006e9c7fc Gerrit-Change-Number: 8609 Gerrit-PatchSet: 1 Gerrit-Owner: Lars Volker
[Impala-ASF-CR] IMPALA-6092: avoid drop/create function interactions in e2e tests
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8593 ) Change subject: IMPALA-6092: avoid drop/create function interactions in e2e tests .. Patch Set 2: Code-Review+1 (1 comment) +1 assuming the new test works on all FSes. http://gerrit.cloudera.org:8080/#/c/8593/1/tests/query_test/test_udfs.py File tests/query_test/test_udfs.py: http://gerrit.cloudera.org:8080/#/c/8593/1/tests/query_test/test_udfs.py@422 PS1, Line 422: check_call(["hadoop", "fs", "-put", "-f", src_udf_path, tgt_udf_path]) > Couple of points. This always copies from a local source and it renames to What I meant was that "hadoop fs -put/copyFromLocal" from what I understand is used to put/copy stuff across file systems and I just wanted to be sure if it works even in local FS mode too (we had many flaky tests in local fs mode in the past). To test locally, export TARGET_FILESYSTEM="local" export WAREHOUSE_LOCATION_PREFIX=/foo source bin/impala-config.sh ./run-tests.py I think you need to have the test-snapshot loaded in the local fs too, so it might not run that smoothly, but you can repro the steps manually, outside of an e2e. -- To view, visit http://gerrit.cloudera.org:8080/8593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ica3538788b1d2ab5e361261e2ade62780b838e65 Gerrit-Change-Number: 8593 Gerrit-PatchSet: 2 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Nov 2017 22:22:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4985: use parquet stats of nested types for dynamic pruning
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8480 ) Change subject: IMPALA-4985: use parquet stats of nested types for dynamic pruning .. Patch Set 4: (12 comments) Looks pretty good, mostly comments on phrasing. http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@179 PS4, Line 179: // A collection type is "required" if it is checked to be not empty. Collection Shrink? The following seems sufficient. // Tuple descriptors of collection slots that have an IsNotEmptyPredicate. See SelectStmt#registerIsNotEmptyPredicates(). Do we really need a new term "required". The meaning of that term is not clear clear to me from this comment. I think my suggested comment above is pretty clear. A collection slot is required if an only if it has an IsNotEmptyPredicate. http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@184 PS4, Line 184: // Analysis adds IsNotEmptyPredicates where needed to enforce required collections. I think this could be misunderstood. IsNotEmptyPredicates are not required, they are purely a perf optimization, they don't really "enforce" anything in the query semantics sense. Suggest shrinking text as indicated above. http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@436 PS4, Line 436: // Checks if slot refers to an array "pos" pseudo-column. nit: we typically use /** */ style class and function comments http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@440 PS4, Line 440: private boolean isArrayPosReference(SlotRef slot) { Move to SlotRef. I generally prefer 'slotRef' instead of 'slot' because the latter could be misunderstood as a SlotDescriptor. http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@442 PS4, Line 442: if (parent != null) { reverse? if (parent == null) return false; http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@533 PS4, Line 533:* Adds nested collections that are required from 'conjuncts' to requiredCollections_. Populates 'requiredCollections_' based on IsNotEmptyPredicates in the given conjuncts. (don't think we need to say more) http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@537 PS4, Line 537:* Example: table T has field A which is of type array>. I don't think examples really belong here because this is not where we are adding the predicates. Instead they should be in SelecmStmt.registerIsNotEmptyPredicates(). http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@548 PS4, Line 548: Preconditions.checkArgument(ref.getDesc().getType().isComplexType()); checkState http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@549 PS4, Line 549: Preconditions.checkArgument(ref.getDesc().getItemTupleDesc() != null); checkState http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@574 PS4, Line 574: // Note: it is conservative (but correct) to depend on a collection to be required. I think we should state what we mean more directly. We rely on analysis to generate IsNotEmptyPredicates where correct, but analysis is conservative and misses some opportunities. http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@586 PS4, Line 586:* collection-typed slot. As conjuncts are seen, mark nested collections that are mark -> collect http://gerrit.cloudera.org:8080/#/c/8480/4/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test File testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test: http://gerrit.cloudera.org:8080/#/c/8480/4/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test@33 PS4, Line 33: aggregation(SUM, NumRowGroups): 2 we should also have planner tests to see how the explain looks -- To view, visit http://gerrit.cloudera.org:8080/8480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0c99e20cb080b504442cd5376ea3e046016158fe Gerrit-Change-Number: 8480 Gerrit-PatchSet: 4 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 20 Nov 2017 22:27:49 + Gerrit-HasCom
[Impala-ASF-CR] IMPALA-4927: Impala should be able to handle invalid input from Sentry
Pranay Singh has posted comments on this change. ( http://gerrit.cloudera.org:8080/8588 ) Change subject: IMPALA-4927: Impala should be able to handle invalid input from Sentry .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG@10 PS1, Line 10: non existent role > Can you clarify what a "non existent role" is? As per my understanding a non existent ROLE is a invalid role that is created by some other CLIENT, Hue in this case (which has been fixed). The role does not have right privileges and causes issues for other valid roles. http://gerrit.cloudera.org:8080/#/c/8588/1//COMMIT_MSG@20 PS1, Line 20: the code was instrumented to have a invalid Role " " and see how the condition > nit: line overflow. Fixed http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@140 PS1, Line 140: // deleted from this set and we are left with the set of privileges that need > nit: line overflow Done http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@142 PS1, Line 142: Set privilegesToRemove = role.getPrivilegeNames(); > Just for my understanding, which exact line is throwing the exception where listRolePrivileges() @ line# 146 will cause issue. http://gerrit.cloudera.org:8080/#/c/8588/1/fe/src/main/java/org/apache/impala/util/SentryProxy.java@169 PS1, Line 169: role.getName() > Can sentry ever return a null role name? The bug says that HUE created a ROLE with " " -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 1 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Pranay Singh Gerrit-Comment-Date: Mon, 20 Nov 2017 22:40:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4927: Impala should be able to handle invalid input from Sentry
Hello Bharath Vissapragada, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8588 to look at the new patch set (#2). Change subject: IMPALA-4927: Impala should be able to handle invalid input from Sentry .. IMPALA-4927: Impala should be able to handle invalid input from Sentry Impala requests a list of roles from Sentry and then asks for privileges for each role. If Sentry returns a non existent role in the first step, then there will be a Java exception in Impala in the second step and the communication with Sentry is aborted. The issue is fixed by handling the exception if an invalid role is found and continue with getting permissions for the rest of the roles. Testing: --- Since invalid role could not be created through impala-shell/Hue interface the code was instrumented to have a invalid Role " " and see how the condition is handled. Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b --- M fe/src/main/java/org/apache/impala/util/SentryProxy.java 1 file changed, 31 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/8588/2 -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 2 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Pranay Singh
[Impala-ASF-CR] IMPALA-4927: Impala should be able to invalid input from Sentry
Hello Bharath Vissapragada, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8588 to look at the new patch set (#3). Change subject: IMPALA-4927: Impala should be able to invalid input from Sentry .. IMPALA-4927: Impala should be able to invalid input from Sentry Impala requests a list of roles from Sentry and then asks for privileges for each role. If Sentry returns a non existent role in the first step, then there will be a Java exception in Impala in the second step and the communication with Sentry is aborted. The issue is fixed by handling the exception if an invalid role is found and continue with getting permissions for the rest of the roles. Testing: --- Since invalid role could not be created through impala-shell/Hue interface the code was instrumented to have a invalid Role " " and see how the condition is handled. Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b --- M fe/src/main/java/org/apache/impala/util/SentryProxy.java 1 file changed, 31 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/8588/3 -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 3 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Pranay Singh
[Impala-ASF-CR] IMPALA-4927: Impala should be able to invalid input from Sentry
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8588 ) Change subject: IMPALA-4927: Impala should be able to invalid input from Sentry .. Patch Set 3: Adding Zach. I think he's seen something similar. (Or not--could be totally different; in which case, sorry!) -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 3 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Mon, 20 Nov 2017 22:46:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5019: Decimal V2 addition
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/8309 ) Change subject: IMPALA-5019: Decimal V2 addition .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/8309/4/be/src/exprs/expr-test.cc@a2402 PS4, Line 2402: > I'm not sure what the intent of the original test case is and why it was wr I think it's fine to leave as is actually because this problem is fixed in IMPALA-6183, look at patch set 5 (it was rebased on top of the fix). After that fix, these tests work as intended I think. -- To view, visit http://gerrit.cloudera.org:8080/8309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I401049c56d910eb1546a178c909c923b01239336 Gerrit-Change-Number: 8309 Gerrit-PatchSet: 4 Gerrit-Owner: Taras Bobrovytsky Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Mon, 20 Nov 2017 22:54:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4591: Bound Kudu client error mem usage
Hello Tim Armstrong, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8464 to look at the new patch set (#4). Change subject: IMPALA-4591: Bound Kudu client error mem usage .. IMPALA-4591: Bound Kudu client error mem usage Previously, Kudu client errors could grow in size unbounded, potentially causing the process to be killed. This patch sets a bound on the mem that can be used for these error messages, with the size determined by the flag 'kudu_error_buffer_size'. If the errors for a Kudu client exceed this size, the query will fail, as some errors will be dropped and we won't be able to tell if all of the errors can be safely ignored. Testing: - Added a custom cluster test that verifies that a query that exceeds the limit fails. Change-Id: I186ddb3f3b5865e08f17dba57cf6640591d06b14 --- M be/src/exec/kudu-table-sink.cc M be/src/exec/kudu-table-sink.h M tests/custom_cluster/test_kudu.py 3 files changed, 45 insertions(+), 26 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/8464/4 -- To view, visit http://gerrit.cloudera.org:8080/8464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I186ddb3f3b5865e08f17dba57cf6640591d06b14 Gerrit-Change-Number: 8464 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4591: Bound Kudu client error mem usage
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/8464 ) Change subject: IMPALA-4591: Bound Kudu client error mem usage .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/8464/2/be/src/exec/kudu-table-sink.cc File be/src/exec/kudu-table-sink.cc: http://gerrit.cloudera.org:8080/#/c/8464/2/be/src/exec/kudu-table-sink.cc@350 PS2, Line 350: mem_tracker_->Release(client_tracked_bytes_); > When you say "cleared" do you mean zeroed or freed? Also, if CountPendingEr Good points. Looking through the Kudu client code, I think null-ing the shared_ptr is the right idea. http://gerrit.cloudera.org:8080/#/c/8464/2/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/8464/2/tests/custom_cluster/test_kudu.py@78 PS2, Line 78: cursor.execute( > But what about when the limit is set to 1024? Is there a way to determine t Good point. I'll add another query to this test that doesn't overflow the limit. -- To view, visit http://gerrit.cloudera.org:8080/8464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I186ddb3f3b5865e08f17dba57cf6640591d06b14 Gerrit-Change-Number: 8464 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Nov 2017 23:00:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Zoram Thanga has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8610 Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. IMPALA-6225: Query profile date-time strings should have ns precision. IPALA-5599 changed the precision of query start and end time date-time string representations to microseconds. This ended up breaking compatibility with some API clients. This patch restores the precision to nanosecond, even though the timestamps themselves have only microsecond precision. Effectively, what we end up doing is to zero-pad the fractional second part to nine decimal places. Change-Id: I95bab8de19d437956f42e13b754ab4dbb52284ca --- M be/src/service/client-request-state.cc 1 file changed, 4 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/8610/1 -- To view, visit http://gerrit.cloudera.org:8080/8610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I95bab8de19d437956f42e13b754ab4dbb52284ca Gerrit-Change-Number: 8610 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga
[Impala-ASF-CR] IMPALA-4927: Impala should handle invalid input from Sentry
Hello Bharath Vissapragada, Philip Zeyliger, Zach Amsden, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8588 to look at the new patch set (#4). Change subject: IMPALA-4927: Impala should handle invalid input from Sentry .. IMPALA-4927: Impala should handle invalid input from Sentry Impala requests a list of roles from Sentry and then asks for privileges for each role. If Sentry returns a non existent role in the first step, then there will be a Java exception in Impala in the second step and the communication with Sentry is aborted. The issue is fixed by handling the exception if an invalid role is found and continue with getting permissions for the rest of the roles. Testing: --- Since invalid role could not be created through impala-shell/Hue interface the code was instrumented to have a invalid Role " " and see how the condition is handled. Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b --- M fe/src/main/java/org/apache/impala/util/SentryProxy.java 1 file changed, 31 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/8588/4 -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 4 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8414 ) Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation .. Patch Set 9: (19 comments) http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/disk-io-mgr.h File be/src/runtime/io/disk-io-mgr.h: http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/disk-io-mgr.h@a222 PS11, Line 222: : : > did you mean to just delete that? Done http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/disk-io-mgr.h@457 PS11, Line 457: XCEED > queued? Done http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/disk-io-mgr.cc File be/src/runtime/io/disk-io-mgr.cc: http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/disk-io-mgr.cc@333 PS11, Line 333: CANCELLED > seems like the right thing to do but are you sure we weren't depending on p Before this patch the only place when RequestContext::Cancel() was called with a non-CANCELLED status was with MEM_LIMIT_EXCEEDED. This patch removes that call and the need for any asynchronous error propagation via RequestContext, so can't introduce any new bugs in this area. Error propagation bugs are still possible in client code if they cancel the context before propagating the original error, but I believe the HDFS scan node code is written to avoid this. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/disk-io-mgr.cc@581 PS11, Line 581: DCHECK(!buffer->is_cached()) << "HDFS cache reads don't go through this code path."; > maybe put that dcheck upstream too (like ReadRange() or even earlier) to he There is already a DCHECK in ReadRange() that catches this, but its non-obvious. I added a string to it to make it clearer what it's checking. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/disk-io-mgr.cc@665 PS11, Line 665: DCHECK > nit: _EQ The strong typed enums prevent that unfortunately. I did change this to print out the integer value on failure. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.h File be/src/runtime/io/request-context.h: http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.h@448 PS11, Line 448: again. > disk threads? Done http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h File be/src/runtime/io/request-context.h: http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h@27 PS9, Line 27: For most I/O manager clients it is an : /// opaque pointer, but some clients may need to include this heade > is that still true? Done http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h@93 PS9, Line 93: void Cancel(); > how did you decide to make this a first class thing in the RequestContext, Yeah, I'd probably call it DiskIoMgrClient or something like that if I was writing this from scratch. The ones I moved were the trivial methods that were only called during RequestContext set-up and tear-down. The other DiskIoMgr methods are called from more places and are more about the I/O operations than setting up the context. It isn't really principled now that I think about it. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc File be/src/runtime/io/request-context.cc: http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc@42 PS11, Line 42: << static_cast(range->external_buffer_tag_); > oh, I guess DCHECK_EQ doesn't work with enum or something? Yeah, not with the strongly-typed "enum class" enums. It works with weakly typed enums because they get implicitly converted to integers. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc@75 PS11, Line 75: reader > what about the writing case? is that relevant? Yep, this needed updating. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc@76 PS11, Line 76: GetNext > GetNext() Done http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc@79 PS11, Line 79: it > does this referring to the context or something else? Done http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc@90 PS11, Line 90: cancelled Status > does that mean the CANCELLED status or the status that lead to cancellation The RequestContext can't be cancelled with an arbitrary status. If there was an earlier error for a ScanRange, the cancelled status doesn't overwrite the prior status. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc@91 PS11, Line 91: the cancelled state. > what does that mean? reworded. http://gerrit.cloudera.org:8080/#/c/8414/11/be/src/runtime/io/request-context.cc@95 PS11, Line 95: decrements the number of disk queues the context : // is on. > which code is this talking about? Oh, I guess that's no
[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
Hello Tianyi Wang, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8414 to look at the new patch set (#12). Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation .. IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation In preparation for switching the I/O mgr to the buffer pool, this removes and cleans up a lot of code so that the switchover patch starts from a cleaner slate. * Remove the free buffer cache (which will be replaced by buffer pool's own caching). * Make memory limit exceeded error checking synchronous (in anticipation of having to propagate buffer pool errors synchronously). * Simplify error propagation - remove the (ineffectual) code that enqueued BufferDescriptors containing error statuses. * Document locking scheme better in a few places, make it part of the function signature when it seemed reasonable. * Move ReturnBuffer() to ScanRange, because it is intrinsically connected with the lifecycle of a scan range. * Separate external ReturnBuffer() and internal CleanUpBuffer() interfaces - previously callers of ReturnBuffer() were fudging the num_buffers_in_reader accounting to make the external interface work. * Eliminate redundant state in ScanRange: 'eosr_returned_' and 'is_cancelled_'. * Clarify the logic around calling Close() for the last BufferDescriptor. -> There appeared to be an implicit assumption that buffers would be freed in the order they were returned from the scan range, so that the "eos" buffer was returned last. Instead just count the number of outstanding buffers to detect the last one. -> Touching the is_cancelled_ field without holding a lock was hard to reason about - violated locking rules and it was unclear that it was race-free. * Remove DiskIoMgr::Read() to simplify interface. It is trivial to inline at the callsites. This will probably regress performance somewhat because of the cache removal, so my plan is to merge it around the same time as switching the I/O mgr to allocate from the buffer pool. I'm keeping the patches separate to make reviewing easier. Testing: * Ran exhaustive tests * Ran the disk-io-mgr-stress-test overnight Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 --- M be/src/exec/hdfs-parquet-scanner.cc M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node.cc M be/src/exec/scanner-context.cc M be/src/runtime/exec-env.cc M be/src/runtime/io/disk-io-mgr-stress.cc M be/src/runtime/io/disk-io-mgr-test.cc M be/src/runtime/io/disk-io-mgr.cc M be/src/runtime/io/disk-io-mgr.h M be/src/runtime/io/request-context.cc M be/src/runtime/io/request-context.h M be/src/runtime/io/request-ranges.h M be/src/runtime/io/scan-range.cc M be/src/runtime/mem-tracker.h M be/src/runtime/test-env.cc M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h 19 files changed, 575 insertions(+), 904 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/8414/12 -- To view, visit http://gerrit.cloudera.org:8080/8414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 Gerrit-Change-Number: 8414 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8414 ) Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h File be/src/runtime/io/request-context.h: http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h@93 PS9, Line 93: void Cancel(); > Yeah, I'd probably call it DiskIoMgrClient or something like that if I was I'm open to how to move forward to this. I can revert this to being more of an opaque pointer if we think it needs more though. -- To view, visit http://gerrit.cloudera.org:8080/8414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 Gerrit-Change-Number: 8414 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Nov 2017 23:26:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4927: Impala should handle invalid input from Sentry
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/8588 ) Change subject: IMPALA-4927: Impala should handle invalid input from Sentry .. Patch Set 4: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/8588/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/8588/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@169 PS4, Line 169: value nit: remove. -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 4 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Mon, 20 Nov 2017 23:28:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8414 ) Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h File be/src/runtime/io/request-context.h: http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h@93 PS9, Line 93: void Cancel(); > I'm open to how to move forward to this. I can revert this to being more of Let me take a look through the current patchset first. -- To view, visit http://gerrit.cloudera.org:8080/8414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 Gerrit-Change-Number: 8414 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 20 Nov 2017 23:34:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1144: Fix exception when cancelling query in Impala-shell with CTRL-C
Hello Lars Volker, Laszlo Gaal, Michael Brown, Zoltan Borok-Nagy, Philip Zeyliger, David Knupp, Attila Jeges, Tim Armstrong, Csaba Ringhofer, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8549 to look at the new patch set (#6). Change subject: IMPALA-1144: Fix exception when cancelling query in Impala-shell with CTRL-C .. IMPALA-1144: Fix exception when cancelling query in Impala-shell with CTRL-C Issue1: When query is cancelled via CTRL-C while being executed in Impala-shell then an exception is thrown from Impala backend saying 'Invalid query handle'. This is because one ImpalaClient was making RPC's while another ImpalaClient cancelled the query on the backend. As a result RPC handlers in ImpalaServer try to access a ClientRequestState that had been cleared from the backend. The issue is confidently reproducable both in wait_to_finish and in fetch states of the query. As a solution the query cancellation is indicated to ImpalaClient via a bool flag. Once a cancellation originated exception reaches Impala shell this flag is checked to decide whether to suppress the error or not. Issue2: Every time a query was cancelled a 'use db' command was issued automatically. This happened to historical reasons but is not needed anymore (see Jira for more details). Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 --- M shell/impala_client.py M shell/impala_shell.py M tests/shell/test_shell_commandline.py 3 files changed, 56 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/49/8549/6 -- To view, visit http://gerrit.cloudera.org:8080/8549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 Gerrit-Change-Number: 8549 Gerrit-PatchSet: 6 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction
Hello Lars Volker, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8436 to look at the new patch set (#7). Change subject: IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction .. IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction During dictionary constructon, most types are copied from the parquet dictionary page, but StringValues keep pointers to it. In this case, the dictionary page must be kept and attached to the last row batch that references it. In case of other types, it is safe do delete the dictionary page after the construction of the dictionary. This patch contains two optimizations: - dictionary pages are deleted as soon as possible for non string types - in non-compressed and non-string case, an unnecessary copy is avoided Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 --- M be/src/exec/parquet-column-readers.cc 1 file changed, 33 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/8436/7 -- To view, visit http://gerrit.cloudera.org:8080/8436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Gerrit-Change-Number: 8436 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1144: Fix exception when cancelling query in Impala-shell with CTRL-C
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/8549 ) Change subject: IMPALA-1144: Fix exception when cancelling query in Impala-shell with CTRL-C .. Patch Set 6: (10 comments) http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8549/2//COMMIT_MSG@25 PS2, Line 25: Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 > It seems ok that it doesn't reproduce it every time - it's inherently racy. The test reproducing this seems pretty stable. I'll run the production/ASAN tests, sure. http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@58 PS5, Line 58: QueryCancelledByShellEx > Can we call this QueryCancelledByShellException or ExpectedQueryCancellatio Sure, I'll go for QueryCancelledByShellException. http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@60 PS5, Line 60: class ImpalaClient(object): > The value might not be necessary if we're always going to pass in the same Done http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@83 PS5, Line 83: > Can you add a brief comment explaining how this works. I.e. that it's set t Done http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_client.py@435 PS5, Line 435: if suppress_error_on_cancel and self.is_query_cancelled: > It's weird that we don't need the logic for QueryNotFoundException. Does Im I haven't found any usage in the non-generated part of Impala. I'll add the cancellation check here, though to make consistent. http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8549/5/shell/impala_shell.py@1026 PS5, Line 1026: (verb, num_rows, error_report, time_elapsed)) > It might be to just not print anything - we don't always throw this excepti Done http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@333 PS5, Line 333: 'select * from v, v v2, v v3, v v4, v v5, v v6, v v7, v v8, ' + \ > Maybe add a few more tables to the cross join to make sure that it runs for Well, this already runs for more than 20s but I see your point, we can be on the safe side adding more tables. With 2 more tables the runtime increases over 5min :) Done http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@342 PS5, Line 342: args = '-q "select * from tpch.customer c1, tpch.customer c2, ' + \ > You could also do something like: I think this query should be fine. I gave it a try and after ~30 mins it still haven't reached the fetching state. http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@346 PS5, Line 346: > Isn't this the command-line arguments? Done http://gerrit.cloudera.org:8080/#/c/8549/5/tests/shell/test_shell_commandline.py@349 PS5, Line 349: execution in fact starts and then cancels it. Expects the query > Can you add a comment to the function explaining that it waits 3 seconds be Sure, I'll add these comments to the function description. In fact, this 3 sec is a magic number I took from the other cancellation tests. I played around a little bit on my local machine and started decreasing the value. Apparently, the turning point is somewhere between 0.2 and 0.1 secs where the test starts failing. To be on the safe side, I decided to go for 0.5sec. Do you think it's reasonable? (I'll kick of a release and asan test in Jenkins to see how they behave with this value) -- To view, visit http://gerrit.cloudera.org:8080/8549 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6cefaf1dae78baae238289816a7cb9d210fb38e2 Gerrit-Change-Number: 8549 Gerrit-PatchSet: 6 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Laszlo Gaal Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 20 Nov 2017 23:51:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8610 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 1: Thanks! I think tests/query_test/test_observability.py may be the cheapest place to add a test for this. I think it'd be useful to add a quick test, to help us think twice about changing this in the future. Code itself looks straight-forward to me. -- To view, visit http://gerrit.cloudera.org:8080/8610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95bab8de19d437956f42e13b754ab4dbb52284ca Gerrit-Change-Number: 8610 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Mon, 20 Nov 2017 23:57:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/8436 ) Change subject: IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction .. Patch Set 8: > Uploaded patch set 7. Oops, there was a rebase... to see the changes that fix CHAR type handling, see diff between patch 6 and 8. Tim, can you rerun the tests for me? -- To view, visit http://gerrit.cloudera.org:8080/8436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Gerrit-Change-Number: 8436 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Nov 2017 00:04:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8610 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 1: As discussed offline, it may be safe to undo the changes in time precision in some other locations (e.g. ImpalaHttpHandler::QueryStateToJson) to be consistent with what the profile is showing. -- To view, visit http://gerrit.cloudera.org:8080/8610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95bab8de19d437956f42e13b754ab4dbb52284ca Gerrit-Change-Number: 8610 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Nov 2017 00:00:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/6023 ) Change subject: IMPALA-4848: Add WIDTH_BUCKET() function .. Patch Set 9: (20 comments) http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/expr-test.cc@4653 PS9, Line 4653: TestValue("width_bucket(22, 5, 20.1, 4)", TYPE_BIGINT, 5); Add some tests with nulls. For example: (NULL, 5, 20, 3) (12, NULL, 20, 3) (12, 5, NULL, 3) (12, 5, 20, NULL) http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc File be/src/exprs/math-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@455 PS9, Line 455: min_range = -1 (or any negative value) You can just write: min_range < 0 http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460 PS9, Line 460: Incase "In case" http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460 PS9, Line 460: t extra space here http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@460 PS9, Line 460: functions function* http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@461 PS9, Line 461: decimal8Value I am not sure it makes sense to even mention decimal8Value here. Why would you even think about storing it in a decimal 8? http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@461 PS9, Line 461: stroring spelling http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@480 PS9, Line 480: if (min_range == max_range) { What if min_range > max_range? Also, add a test case. http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@481 PS9, Line 481: Lower bound cannot be equal to upper bound Would it make sense to include the name of the function in the error message? Do we normally do that? http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@489 PS9, Line 489: num_buckets.val; how about: num_buckets.val + 1 Also, add a test case where num_buckets is a maximally large integer, so adding one causes an overflow. http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@494 PS9, Line 494: // FE casts all the arguments to the same type. Maybe expand this comment a little? E.g "expr", "min_range" and "max_range" are guaranteed to be the same scale and precision by the FE http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@503 PS9, Line 503: if(overflow) { Have we considered what happens when subtraction is successful (and overflow is false), but the result is larger than the largest allowed decimal (10^38 - 1). Add a test case like this. For example: min range = -100 max_range = 10^38 - 1 The result of that subtraction should not overflow, because it fits into int128_t http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@514 PS9, Line 514: if(overflow) { Can this overflow ever happen without overflowing on line 503? Add a test case for this overflow and error message. (You might need to add some End to end tests to test for the error message) http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@533 PS9, Line 533: if(needs_int256) { Nit: Put a space between "if" and the opening brace. (here and elsewhere) http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@539 PS9, Line 539: avoid avoid written twice http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@549 PS9, Line 549: // bump up the denominator scale so that the scale of the numerator and denominator Can you explain this a little better? It's not really clear why we have to scale it up. http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@551 PS9, Line 551: int128_t y = DecimalUtil::MultiplyByScale(range_size.value(), Is it possible for this to overflow? http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@557 PS9, Line 557: ctx->SetError("Overflow while dividing by the range_size"); This should be moved up to around line 543. Add a test case for this error message. http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@571 PS9, Line 571: if (UNLIKELY(num_buckets.val <= 0)) { Why add this check here, instead of around line 480 where other similar check are located? http://gerrit.cloudera.org:8080/#/c/6023/9/fe/src/main/java/org/apache/impala/analysis/Expr.java File fe/src/main/java/org/apache/impala/analysis/Expr.java: http://gerrit.cloudera.org:8080/#/c/6023/9/fe/src/main/java/org/apache/impala/analysis/Expr.java@459 PS9, Line 459: Why this empty line? -- To view, visit http://gerrit.cloudera.org:8080/6023 To unsubscribe, visit http://gerrit.cloudera
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Zoram Thanga has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/8611 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. IMPALA-6225: Query profile date-time strings should have ns precision. IMPALA-5599 changed the precision of query start and end time date-time string representations to microseconds. This ended up breaking compatibility with some API clients. This patch restores the precision to nanosecond, even though the timestamps themselves have only microsecond precision. Effectively, what we end up doing is to zero-pad the fractional second part to nine decimal places. I have manually checked from the Impala debug web page that the start and end times of queries have nanosecond precision: Start Time: 2017-11-20 14:59:01.954031000 End Time: 2017-11-20 15:00:02.103735000 This is basically the same as how it was before. The following is taken from a cluster running Impala 2.11: Start Time: 2017-11-20 14:17:52.19827 End Time: 2017-11-20 14:18:52.242868000 Change-Id: I2e124b9c7e0717b8dc2cdab46aea41d74c5f2fd0 --- M be/src/service/client-request-state.cc M be/src/service/impala-http-handler.cc M be/src/util/time.h 3 files changed, 10 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/8611/2 -- To view, visit http://gerrit.cloudera.org:8080/8611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2e124b9c7e0717b8dc2cdab46aea41d74c5f2fd0 Gerrit-Change-Number: 8611 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/8610 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 1: Looking into the test code, thanks Phil for the suggestion. Uploading the second patch for the product code change. -- To view, visit http://gerrit.cloudera.org:8080/8610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95bab8de19d437956f42e13b754ab4dbb52284ca Gerrit-Change-Number: 8610 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Nov 2017 00:19:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/8611 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 2: Sorry, I messed up the gerrit ID, so here we are. I will abandon the other change id. -- To view, visit http://gerrit.cloudera.org:8080/8611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e124b9c7e0717b8dc2cdab46aea41d74c5f2fd0 Gerrit-Change-Number: 8611 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Nov 2017 00:22:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/8610 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 1: Please see https://gerrit.cloudera.org/#/c/8611/ instead. -- To view, visit http://gerrit.cloudera.org:8080/8610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95bab8de19d437956f42e13b754ab4dbb52284ca Gerrit-Change-Number: 8610 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Nov 2017 00:23:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4985: use parquet stats of nested types for dynamic pruning
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/8480 ) Change subject: IMPALA-4985: use parquet stats of nested types for dynamic pruning .. Patch Set 4: (12 comments) http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@179 PS4, Line 179: // A collection type is "required" if it is checked to be not empty. Collection > Shrink? The following seems sufficient. went with just referring to not-empty instead of not-empty and required. http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@184 PS4, Line 184: // Analysis adds IsNotEmptyPredicates where needed to enforce required collections. > I think this could be misunderstood. IsNotEmptyPredicates are not required, Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@436 PS4, Line 436: // Checks if slot refers to an array "pos" pseudo-column. > nit: we typically use /** */ style class and function comments whoops, Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@440 PS4, Line 440: private boolean isArrayPosReference(SlotRef slot) { > Move to SlotRef. I generally prefer 'slotRef' instead of 'slot' because the Done. sync'd with similar uses below as well. http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@442 PS4, Line 442: if (parent != null) { > reverse? if (parent == null) return false; Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@533 PS4, Line 533:* Adds nested collections that are required from 'conjuncts' to requiredCollections_. > Populates 'requiredCollections_' based on IsNotEmptyPredicates in the given Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@537 PS4, Line 537:* Example: table T has field A which is of type array>. > I don't think examples really belong here because this is not where we are Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@548 PS4, Line 548: Preconditions.checkArgument(ref.getDesc().getType().isComplexType()); > checkState Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@549 PS4, Line 549: Preconditions.checkArgument(ref.getDesc().getItemTupleDesc() != null); > checkState Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@574 PS4, Line 574: // Note: it is conservative (but correct) to depend on a collection to be required. > I think we should state what we mean more directly. We rely on analysis to Done http://gerrit.cloudera.org:8080/#/c/8480/4/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@586 PS4, Line 586:* collection-typed slot. As conjuncts are seen, mark nested collections that are > mark -> collect Done http://gerrit.cloudera.org:8080/#/c/8480/4/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test File testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test: http://gerrit.cloudera.org:8080/#/c/8480/4/testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test@33 PS4, Line 33: aggregation(SUM, NumRowGroups): 2 > we should also have planner tests to see how the explain looks Done -- To view, visit http://gerrit.cloudera.org:8080/8480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0c99e20cb080b504442cd5376ea3e046016158fe Gerrit-Change-Number: 8480 Gerrit-PatchSet: 4 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 21 Nov 2017 00:33:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4985: use parquet stats of nested types for dynamic pruning
Hello Lars Volker, Alex Behm, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8480 to look at the new patch set (#5). Change subject: IMPALA-4985: use parquet stats of nested types for dynamic pruning .. IMPALA-4985: use parquet stats of nested types for dynamic pruning Currently, parquet row-groups can be pruned at run-time using min/max stats when predicates (in, binary) are specified for column scalar types. This patch extends pruning to nested types for the same class of predicates. A nested value is an instance of a nested type (struct, array, map). A nested value consists of other nested and scalar values (as declared by its type). Predicates that can be used for row-group pruning must be applied to nested scalar values. In addition, the parent of the nested scalar must also be required, that is, not empty. The latter requirement is conservative: some filters that could be used for pruning are not used for correctness reasons. Testing: - extended nested-types-parquet-stats e2e test cases. Change-Id: I0c99e20cb080b504442cd5376ea3e046016158fe --- M be/src/exec/hdfs-parquet-scanner.h M fe/src/main/java/org/apache/impala/analysis/CollectionStructType.java M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test M testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test M tests/query_test/test_nested_types.py 7 files changed, 508 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/8480/5 -- To view, visit http://gerrit.cloudera.org:8080/8480 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0c99e20cb080b504442cd5376ea3e046016158fe Gerrit-Change-Number: 8480 Gerrit-PatchSet: 5 Gerrit-Owner: Vuk Ercegovac Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8611 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 2: As mentioned before, an alternate approach to go back to the previous behavior is to change the To*StringFromUnix* functions to use nano-second precision by default. That will make it consistent across the entire code base unless otherwise specified by the callers. -- To view, visit http://gerrit.cloudera.org:8080/8611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e124b9c7e0717b8dc2cdab46aea41d74c5f2fd0 Gerrit-Change-Number: 8611 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Nov 2017 00:45:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4927: Impala should handle invalid input from Sentry
Zach Amsden has posted comments on this change. ( http://gerrit.cloudera.org:8080/8588 ) Change subject: IMPALA-4927: Impala should handle invalid input from Sentry .. Patch Set 4: (3 comments) What version of Sentry was causing this issue? Also, what was the exact exception being throw? http://gerrit.cloudera.org:8080/#/c/8588/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java File fe/src/main/java/org/apache/impala/util/SentryProxy.java: http://gerrit.cloudera.org:8080/#/c/8588/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@141 PS4, Line 141: // need to be removed. It's fine to leave this as is, the 90 column limit is not exceeded. http://gerrit.cloudera.org:8080/#/c/8588/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@146 PS4, Line 146: sentryPolicyService_.listRolePrivileges(processUser_, role.getName())) { I think it would be better to call listRolePrivileges directly and catch exceptions from that instead of at the end of the function. If the role doesn't exist anymore, we definitely want to remove all its privileges from the catalog instead of going on to exception handling. http://gerrit.cloudera.org:8080/#/c/8588/4/fe/src/main/java/org/apache/impala/util/SentryProxy.java@168 PS4, Line 168: } catch (Exception e) { Can you catch a more specific exception? For example, we probably wouldn't want to catch a NullPointerException here. -- To view, visit http://gerrit.cloudera.org:8080/8588 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I781411018d580854d80a9cad81a1ded7ca16af8b Gerrit-Change-Number: 8588 Gerrit-PatchSet: 4 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zach Amsden Gerrit-Comment-Date: Tue, 21 Nov 2017 00:57:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-2181: Add query option levels for display
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8447 ) Change subject: IMPALA-2181: Add query option levels for display .. Patch Set 15: (17 comments) http://gerrit.cloudera.org:8080/#/c/8447/14//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8447/14//COMMIT_MSG@10 PS14, Line 10: diplayed displayed http://gerrit.cloudera.org:8080/#/c/8447/14//COMMIT_MSG@12 PS14, Line 12: called SET ALL shows all the options grouped by their option levels. Mention that there are also server-side versions of these commands? http://gerrit.cloudera.org:8080/#/c/8447/14/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/8447/14/be/src/service/impala-server.cc@1236 PS14, Line 1236: else { Nit: } else { http://gerrit.cloudera.org:8080/#/c/8447/15/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/8447/15/be/src/service/query-options.h@45 PS15, Line 45: QUERY_OPT_FN(disable_cached_reads, DISABLE_CACHED_READS, TQueryOptionLevel::REGULAR)\ Looks like disable_cached_reads is deprecated: see IMPALA-2963 . I missed that on an earlier pass http://gerrit.cloudera.org:8080/#/c/8447/15/be/src/service/query-options.h@89 PS15, Line 89: QUERY_OPT_FN(enable_expr_rewrites, ENABLE_EXPR_REWRITES, TQueryOptionLevel::REGULAR)\ I think enable_expr_rewrites should be advanced. It's only disabled as a workaround to planner bugs. http://gerrit.cloudera.org:8080/#/c/8447/15/be/src/service/query-options.h@91 PS15, Line 91: QUERY_OPT_FN(parquet_dictionary_filtering, PARQUET_DICTIONARY_FILTERING, TQueryOptionLevel::REGULAR)\ parquet_dictionary_filtering should probably be advanced, there's typically not much reason to disable it unless working around a bug. http://gerrit.cloudera.org:8080/#/c/8447/15/be/src/service/query-options.h@93 PS15, Line 93: QUERY_OPT_FN(parquet_read_statistics, PARQUET_READ_STATISTICS, TQueryOptionLevel::REGULAR)\ Same for parquet_read_statistics. http://gerrit.cloudera.org:8080/#/c/8447/14/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/8447/14/be/src/service/query-options.h@39 PS14, Line 39: QUERY_OPT_FN(abort_on_default_limit_exceeded, ABORT_ON_DEFAULT_LIMIT_EXCEEDED, TQueryOptionLevel::DEPRECATED)\ nit: can we wrap these to 90 chars? http://gerrit.cloudera.org:8080/#/c/8447/14/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/8447/14/be/src/service/query-options.cc@41 PS14, Line 41: using namespace beeswax; nit: probably best to individually import the names so it's easier to figure out what got imported. http://gerrit.cloudera.org:8080/#/c/8447/14/common/thrift/ImpalaService.thrift File common/thrift/ImpalaService.thrift: http://gerrit.cloudera.org:8080/#/c/8447/14/common/thrift/ImpalaService.thrift@294 PS14, Line 294: typedef map TQueryOptionLevels Does this need to be a typedef here? I don't see any references in any thrift files. Maybe we can just use the map type directly where its needed in the backend http://gerrit.cloudera.org:8080/#/c/8447/14/common/thrift/beeswax.thrift File common/thrift/beeswax.thrift: http://gerrit.cloudera.org:8080/#/c/8447/14/common/thrift/beeswax.thrift@111 PS14, Line 111: 4: optional TQueryOptionLevel level Comment that this was added for impala-shell? http://gerrit.cloudera.org:8080/#/c/8447/14/fe/src/main/java/org/apache/impala/analysis/SetStmt.java File fe/src/main/java/org/apache/impala/analysis/SetStmt.java: http://gerrit.cloudera.org:8080/#/c/8447/14/fe/src/main/java/org/apache/impala/analysis/SetStmt.java@55 PS14, Line 55: if (isSetAll_) { Nit: could be one line if (isSetAll_) return ...; http://gerrit.cloudera.org:8080/#/c/8447/14/fe/src/main/java/org/apache/impala/analysis/SetStmt.java@76 PS14, Line 76: request.setIs_set_all(true); nit: could be one line http://gerrit.cloudera.org:8080/#/c/8447/14/shell/impala_client.py File shell/impala_client.py: http://gerrit.cloudera.org:8080/#/c/8447/14/shell/impala_client.py@97 PS14, Line 97: self.query_option_levels[option.key.upper()] = option.level Does this still work if we're talking to an older Impala server that doesn't return the level? Update: looks like it doesn't: Traceback (most recent call last): File "/home/tarmstrong/Impala/incubator-impala/shell/impala_shell.py", line 15 27, in shell = ImpalaShell(options, query_options) File "/home/tarmstrong/Impala/incubator-impala/shell/impala_shell.py", line 20 5, in __init__ self.do_connect(options.impalad) File "/home/tarmstrong/Impala/incubator-impala/shell/impala_shell.py", line 73 1, in do_connect self.imp_client.build_default_query_options_dict() File "/home/tarmstrong/Impala/incubator-impala/shell/impala_client.py", line 9 7, in build_defaul
[Impala-ASF-CR] IMPALA-4591: Bound Kudu client error mem usage
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8464 ) Change subject: IMPALA-4591: Bound Kudu client error mem usage .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I186ddb3f3b5865e08f17dba57cf6640591d06b14 Gerrit-Change-Number: 8464 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Nov 2017 01:07:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Michael Ho has abandoned this change. ( http://gerrit.cloudera.org:8080/8610 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Abandoned Dup with https://gerrit.cloudera.org/#/c/8611/ -- To view, visit http://gerrit.cloudera.org:8080/8610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I95bab8de19d437956f42e13b754ab4dbb52284ca Gerrit-Change-Number: 8610 Gerrit-PatchSet: 1 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8436 ) Change subject: IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Gerrit-Change-Number: 8436 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Nov 2017 01:12:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8436 ) Change subject: IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1504/ -- To view, visit http://gerrit.cloudera.org:8080/8436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Gerrit-Change-Number: 8436 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Nov 2017 01:12:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/8611 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 2: Code-Review+1 (1 comment) Please add a test as Phil suggested. If we decide to add 64-bit timestamps to the profile, please file a JIRA for it. http://gerrit.cloudera.org:8080/#/c/8611/2/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/8611/2/be/src/service/client-request-state.cc@110 PS2, Line 110: summary_profile_->AddInfoString("Start Time", ToStringFromUnixMicros(start_time_us(), : TimePrecision::Nanosecond)); Please add a comment as to why we use nano-second here: Some management software may rely on parsing the "Start Time" and "End Time" to derive duration and other metrics. To avoid breaking compatibility, keep to nano-seconds for timestamp string for now. TODO: Provide 64-bit timestamp for start and end time in nano-second so consumers of profiles can derive it properly. -- To view, visit http://gerrit.cloudera.org:8080/8611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e124b9c7e0717b8dc2cdab46aea41d74c5f2fd0 Gerrit-Change-Number: 8611 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Nov 2017 01:15:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1503/ -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Nov 2017 01:30:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4132: Use -fno-omit-frame-pointer
Gabor Kaszab has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8612 Change subject: IMPALA-4132: Use -fno-omit-frame-pointer .. IMPALA-4132: Use -fno-omit-frame-pointer Using -fno-omit-frame-pointer would keep the frame pointers for functions in the register. As a result we expect more useful stack traces to be produced. For testing performance benchmark was executed on TPC-H and TPC-DS without any significant discrepancy from the baseline results. (For the specific measures check the attachments in the Jira.) Change-Id: Ib7d0d88ba015a847356ed0274fe91017b98cb941 --- M be/CMakeLists.txt 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/12/8612/1 -- To view, visit http://gerrit.cloudera.org:8080/8612 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib7d0d88ba015a847356ed0274fe91017b98cb941 Gerrit-Change-Number: 8612 Gerrit-PatchSet: 1 Gerrit-Owner: Gabor Kaszab
[Impala-ASF-CR] IMPALA-4848: Add WIDTH BUCKET() function
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/6023 ) Change subject: IMPALA-4848: Add WIDTH_BUCKET() function .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc File be/src/exprs/math-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/6023/9/be/src/exprs/math-functions-ir.cc@503 PS9, Line 503: if(overflow) { > Have we considered what happens when subtraction is successful (and overflo Actually I'm not correct here. It does overflow in the case I provided. Wouldn't hurt to add the test case anyways. -- To view, visit http://gerrit.cloudera.org:8080/6023 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I081bc916b1bef7b929ca161a9aade3b54c6b858f Gerrit-Change-Number: 6023 Gerrit-PatchSet: 9 Gerrit-Owner: anujphadke Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: anujphadke Gerrit-Comment-Date: Tue, 21 Nov 2017 01:43:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 7: Flaky test, will restart the Jenkins job. -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Nov 2017 01:56:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1505/ -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Nov 2017 01:56:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4506: Do not display "tip of the day" if --quiet is set
Kim Jin Chul has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8613 Change subject: IMPALA-4506: Do not display "tip of the day" if --quiet is set .. IMPALA-4506: Do not display "tip of the day" if --quiet is set Change-Id: I19c6d00dfbbe805ee9c525b72eb5703840e2f582 --- M shell/impala_shell.py 1 file changed, 10 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8613/1 -- To view, visit http://gerrit.cloudera.org:8080/8613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I19c6d00dfbbe805ee9c525b72eb5703840e2f582 Gerrit-Change-Number: 8613 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul
[Impala-ASF-CR] IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds
Kim Jin Chul has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8614 Change subject: IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds .. IMPALA-2250: Make multiple COUNT(DISTINCT) message state workarounds Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89 --- M fe/src/main/java/org/apache/impala/analysis/AggregateInfo.java 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/8614/1 -- To view, visit http://gerrit.cloudera.org:8080/8614 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5084be10946d68f3ec0760c2b7e698635df26a89 Gerrit-Change-Number: 8614 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul
[Impala-ASF-CR] IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8414 ) Change subject: IMPALA-4835: Part 1: simplify I/O mgr mem mgmt and cancellation .. Patch Set 12: Code-Review+2 (1 comment) Please see if Tianyi has any more comments too. http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h File be/src/runtime/io/request-context.h: http://gerrit.cloudera.org:8080/#/c/8414/9/be/src/runtime/io/request-context.h@93 PS9, Line 93: void Cancel(); > Let me take a look through the current patchset first. Even the other methods seem to operate entirely (or almost entirely?) on the 'reader' state, rather than the diskiomgr 'this' itself. Even the lock is the reader lock. So it seems they are more operations on the context. I think the current change is okay as is, but i think we could consider moving even more things into here in a purely mechanical change in the future. -- To view, visit http://gerrit.cloudera.org:8080/8414 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If5cb42437d11c13bc4a55c3ab426b66777332bd1 Gerrit-Change-Number: 8414 Gerrit-PatchSet: 12 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Nov 2017 04:27:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6225: Query profile date-time strings should have ns precision.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8611 ) Change subject: IMPALA-6225: Query profile date-time strings should have ns precision. .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/8611/2/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/8611/2/be/src/service/client-request-state.cc@110 PS2, Line 110: summary_profile_->AddInfoString("Start Time", ToStringFromUnixMicros(start_time_us(), : TimePrecision::Nanosecond)); > Please add a comment as to why we use nano-second here: I'm not sure that TODO is the right thing because then the units are even less clear. At least with the current way, the units are obvious -- it's just that the consumer's parser is being lame. -- To view, visit http://gerrit.cloudera.org:8080/8611 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e124b9c7e0717b8dc2cdab46aea41d74c5f2fd0 Gerrit-Change-Number: 8611 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Nov 2017 04:37:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8436 ) Change subject: IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction .. IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction During dictionary constructon, most types are copied from the parquet dictionary page, but StringValues keep pointers to it. In this case, the dictionary page must be kept and attached to the last row batch that references it. In case of other types, it is safe do delete the dictionary page after the construction of the dictionary. This patch contains two optimizations: - dictionary pages are deleted as soon as possible for non string types - in non-compressed and non-string case, an unnecessary copy is avoided Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Reviewed-on: http://gerrit.cloudera.org:8080/8436 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/exec/parquet-column-readers.cc 1 file changed, 33 insertions(+), 18 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Gerrit-Change-Number: 8436 Gerrit-PatchSet: 10 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8436 ) Change subject: IMPALA-6054: Parquet dictionary pages should be freed on dictionary construction .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8436 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d9d5f4da1028d961155dafdac0028a1c3641004 Gerrit-Change-Number: 8436 Gerrit-PatchSet: 9 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 21 Nov 2017 04:40:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4506: Do not display "tip of the day" if --quiet is set
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8613 ) Change subject: IMPALA-4506: Do not display "tip of the day" if --quiet is set .. Patch Set 1: (1 comment) Thanks for the contribution! I think the code looks fine, but we can go even farther and remove the 'intro'/'welcome' entirely if --quiet is set, or at least that would be my preference. http://gerrit.cloudera.org:8080/#/c/8613/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8613/1/shell/impala_shell.py@1481 PS1, Line 1481: intro = get_welcome_string(options.verbose) Others may have a different opinion, but I don't think we should show the 'intro' or 'welcome string' at all if --quiet is set. -- To view, visit http://gerrit.cloudera.org:8080/8613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19c6d00dfbbe805ee9c525b72eb5703840e2f582 Gerrit-Change-Number: 8613 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Nov 2017 04:56:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 7 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Nov 2017 05:28:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8546 ) Change subject: IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo .. IMPALA-5624: Replace "ls -l" with opendir() in ProcessStateInfo Running shell commands from impalad can be problematic, because using popen() leads to forking which causes a spike in virtual memory. To avoid this, "ls" is replaced with POSIX API calls. FileDescriptorMap fd_desc_ was only used to get the number of file descriptors, so it was unneccesery work to initialize it. It is removed, and only the number of file descriptors is computed. The automatic test for this function is only a sanity check, because there is no way to know the "expected value" in advance, and the number of file desciptors can change anytime. Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Reviewed-on: http://gerrit.cloudera.org:8080/8546 Reviewed-by: Lars Volker Tested-by: Impala Public Jenkins --- M be/src/util/proc-info-test.cc M be/src/util/process-state-info.cc M be/src/util/process-state-info.h 3 files changed, 35 insertions(+), 44 deletions(-) Approvals: Lars Volker: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8546 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ibffae8069a62e100abbfa7d558b49040b095ddc0 Gerrit-Change-Number: 8546 Gerrit-PatchSet: 8 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-4506: Do not display some intro message if --quiet is set
Hello Philip Zeyliger, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8613 to look at the new patch set (#2). Change subject: IMPALA-4506: Do not display some intro message if --quiet is set .. IMPALA-4506: Do not display some intro message if --quiet is set PS#1: Do not display "tip of day" if --quiet is set PS#2: Do not display some intro message if -- quiet is set Change-Id: I19c6d00dfbbe805ee9c525b72eb5703840e2f582 --- M shell/impala_shell.py 1 file changed, 20 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/8613/2 -- To view, visit http://gerrit.cloudera.org:8080/8613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I19c6d00dfbbe805ee9c525b72eb5703840e2f582 Gerrit-Change-Number: 8613 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-4506: Do not display some intro message if --quiet is set
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8613 ) Change subject: IMPALA-4506: Do not display some intro message if --quiet is set .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/8613/1/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8613/1/shell/impala_shell.py@1481 PS1, Line 1481: intro = get_welcome_string(options.verbose) > Others may have a different opinion, but I don't think we should show the ' Your idea has been introduced by PS#2. Let's wait for other's opinions. -- To view, visit http://gerrit.cloudera.org:8080/8613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19c6d00dfbbe805ee9c525b72eb5703840e2f582 Gerrit-Change-Number: 8613 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Kim Jin Chul Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Tue, 21 Nov 2017 05:52:04 + Gerrit-HasComments: Yes