[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 ) Change subject: IMPALA-7488: Fix hang in test_cancellation .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3176/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2 Gerrit-Change-Number: 11465 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 10:03:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 ) Change subject: IMPALA-7488: Fix hang in test_cancellation .. Patch Set 2: The build failure seems to be unrelated, so I have restarted the job. Good catch btw. -- To view, visit http://gerrit.cloudera.org:8080/11465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2 Gerrit-Change-Number: 11465 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 10:06:01 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "IMPALA-7477: Batch-oriented query set construction"
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11462 ) Change subject: Revert "IMPALA-7477: Batch-oriented query set construction" .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I533c12f1b4cfc5b4a372ba834913975b5c52c5a8 Gerrit-Change-Number: 11462 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 19 Sep 2018 10:15:58 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "IMPALA-7477: Batch-oriented query set construction"
Zoltan Borok-Nagy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11462 ) Change subject: Revert "IMPALA-7477: Batch-oriented query set construction" .. Revert "IMPALA-7477: Batch-oriented query set construction" This has been implicated in some incorrect results involving nulls in HS2. This reverts commit b288a6af2eda9631b2bad91896ae4bfd2a3fdf30. Change-Id: I533c12f1b4cfc5b4a372ba834913975b5c52c5a8 Reviewed-on: http://gerrit.cloudera.org:8080/11462 Tested-by: Impala Public Jenkins Reviewed-by: Zoltan Borok-Nagy --- M be/src/exec/plan-root-sink.cc M be/src/exec/plan-root-sink.h M be/src/service/hs2-util.cc M be/src/service/hs2-util.h M be/src/service/query-result-set.cc M be/src/service/query-result-set.h 6 files changed, 161 insertions(+), 320 deletions(-) Approvals: Impala Public Jenkins: Verified Zoltan Borok-Nagy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/11462 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I533c12f1b4cfc5b4a372ba834913975b5c52c5a8 Gerrit-Change-Number: 11462 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11465 ) Change subject: IMPALA-7488: Fix hang in test_cancellation .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2 Gerrit-Change-Number: 11465 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 13:24:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7488: Fix hang in test cancellation
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11465 ) Change subject: IMPALA-7488: Fix hang in test_cancellation .. IMPALA-7488: Fix hang in test_cancellation test_cancellation runs a impala-shell process with a query specified then sends a SIGINT and confirms that the shell cancels the query and exits. The hang was happening because the shell's signal handler was incorrectly using the same Thirft connection when calling Close() as the main shell thread, which is not thread safe. Testing: - Ran test_cancellation in a loop 500 times. Previously the hang would repro about every 10 runs. Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2 Reviewed-on: http://gerrit.cloudera.org:8080/11465 Reviewed-by: Impala Public Jenkins Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M shell/impala_shell.py 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified Tim Armstrong: Looks good to me, but someone else must approve -- To view, visit http://gerrit.cloudera.org:8080/11465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9c4b570604f7706712eb8e19b1ce69bf35cf15e2 Gerrit-Change-Number: 11465 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism
Hello Tim Armstrong, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11444 to look at the new patch set (#3). Change subject: IMPALA-7556: part 1: handle different file systems via polymorphism .. IMPALA-7556: part 1: handle different file systems via polymorphism This commit reorganizes some parts of the ScanRange class. File operations are handled through the abstract FileReader class. The interface supports positional read that will be needed by IMPALA-5843. The concrete file operations are implemented in sub-classes LocalFileReader and HdfsFileReader. File reader classes are responsible for setting counters and metrics related to file operations. The core logic haven't been changed significantly, but quite a lot code fragments were relocated. Testing: Debug exhaustive tests passed Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82 --- M be/src/runtime/io/CMakeLists.txt M be/src/runtime/io/disk-io-mgr.h A be/src/runtime/io/file-reader.cc A be/src/runtime/io/file-reader.h A be/src/runtime/io/hdfs-file-reader.cc A be/src/runtime/io/hdfs-file-reader.h A be/src/runtime/io/local-file-reader.cc A be/src/runtime/io/local-file-reader.h M be/src/runtime/io/request-context.h M be/src/runtime/io/request-ranges.h M be/src/runtime/io/scan-range.cc 11 files changed, 749 insertions(+), 398 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/11444/3 -- To view, visit http://gerrit.cloudera.org:8080/11444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82 Gerrit-Change-Number: 11444 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/11444 ) Change subject: IMPALA-7556: part 1: handle different file systems via polymorphism .. Patch Set 3: (21 comments) http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/file-reader.h File be/src/runtime/io/file-reader.h: http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/file-reader.h@41 PS2, Line 41: /// Returns number of bytes read by this file reader. > I think we always call SetRequestContext() and SetDiskIoMgr() at the same t I modifed the file readers to use the io mgr and request context objects through 'scan_range_'. http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/file-reader.h@50 PS2, Line 50: bytes act > bytes_read() since it's just a trivial accessor Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/file-reader.h@81 PS2, Line 81: n_range > this is a plain accessor so prefer 'lock()' Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/file-reader.h@93 PS2, Line 93: > const? I don't think this changes after initialization. I changed it to a reference. It can't be a const-ref since we are modifying scan_range_.external_buffer_tag_ in Close(). Also scan_range_.file_string() is a non-const member function. I could create a const version of it, but we pass the returned pointer to functions that take non-const pointers. http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.h File be/src/runtime/io/hdfs-file-reader.h: http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.h@47 PS2, Line 47: bool is_borrowed_fh, uint8_t* buffer, int64_t chunk_size, int* bytes_read); > const, since I don't think this is reassigned Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc File be/src/runtime/io/hdfs-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@108 PS2, Line 108: hdfs_file = borrowed_hdfs_fh->file(); > This part was just a mechanical move, right? Would be helpful to flag the p E.g. should I annotate PS1 with gerrit comments? It is the closest PS to the original version, then people could see the diffs relative to PS1. http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@122 PS2, Line 122: // bytes_read_ is only updated after the while loop > optional: If this part is already touched, it would be nice to extract some Yeah I was thinking about it, but didn't want to do it in the first PS. Done. http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@132 PS2, Line 132: // - if read was successful (current_bytes_read != -1) > line too long (96 > 90) Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@142 PS2, Line 142: req_context_read_timer.Stop(); > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@201 PS2, Line 201: sRead > In the .h Close() is after ReadFromCache(). I would prefer this order in .c Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@203 PS2, Line 203: f (*bytes_r > Variable closed_file looks unnecessary. Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@207 PS2, Line 207: } > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@207 PS2, Line 207: > optional: as this block mainly uses private ScanRange variables, I would ex I moved cached_buffer_ into HdfsFileReader since then. For now I leave it as it is. http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@249 PS2, Line 249: . > I would consider moving (most of) this back to ScanRange::ReadFromCache(). Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@265 PS2, Line 265:<< PrettyPrinter::Print(num_remote_bytes_, TUnit::BYTES) > line too long (101 > 90) Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@334 PS2, Line 334: > nit: other members have no space after =. Done http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/local-file-reader.h File be/src/runtime/io/local-file-reader.h: http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/local-file-reader.h@40 PS2, Line 40: /// Points to a C FILE object between calls to Open() and Close(), otherwise nullptr. > Maybe briefly document how this fits into the lifecycle, e.g. set in Open() Added comment about it. http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/local-file-reader.cc File be/src/runtime/io/local-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/local-file-reader.cc@100 PS2, Line 100:
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 15:59:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11444 ) Change subject: IMPALA-7556: part 1: handle different file systems via polymorphism .. Patch Set 3: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/709/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia3d3d2d774075008285230606b992603d5be1a82 Gerrit-Change-Number: 11444 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 19 Sep 2018 16:19:18 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 16:51:25 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3177/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 16:51:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11470 Change subject: IMPALA-7306: regression test for non-removed transient updates .. IMPALA-7306: regression test for non-removed transient updates Adds a test for IMPALA-7305 that reproduces the bug by delaying heartbeats and updates. Increased some timeouts in the test because they were hit once after looping for ~12 hours. Testing: Manually reintroduced the bug by commenting out the code that fixed it and confirmed that the test failed. Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc --- M tests/statestore/test_statestore.py 1 file changed, 58 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11470/1 -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7525: [DOCS] SHOW GRANT USER
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11454 ) Change subject: IMPALA-7525: [DOCS] SHOW GRANT USER .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11454 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa39275c595d73b2fba293f77bb21aa3985dfa38 Gerrit-Change-Number: 11454 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 19 Sep 2018 18:06:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11470 ) Change subject: IMPALA-7306: regression test for non-removed transient updates .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/710/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 19 Sep 2018 18:21:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. Patch Set 2: Code-Review+2 (1 comment) Looks good. can you also update the comments at: request-ranges.h:258 request-ranges.h:525 request-ranges.h:546 http://gerrit.cloudera.org:8080/#/c/11464/2/be/src/runtime/io/request-context.cc File be/src/runtime/io/request-context.cc: http://gerrit.cloudera.org:8080/#/c/11464/2/be/src/runtime/io/request-context.cc@256 PS2, Line 256: CANCELLED nit: CONTEXT_CANCELLED -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 18:40:49 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7579: fix test query profile contains all events on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11461 ) Change subject: IMPALA-7579: fix test_query_profile_contains_all_events on S3 .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11461 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c6e0899455dd4e12636b959fab4bde79f02fb95 Gerrit-Change-Number: 11461 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 18:41:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7579: fix test query profile contains all events on S3
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11461 ) Change subject: IMPALA-7579: fix test_query_profile_contains_all_events on S3 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11461 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c6e0899455dd4e12636b959fab4bde79f02fb95 Gerrit-Change-Number: 11461 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 18:41:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7579: fix test query profile contains all events on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11461 ) Change subject: IMPALA-7579: fix test_query_profile_contains_all_events on S3 .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3178/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11461 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c6e0899455dd4e12636b959fab4bde79f02fb95 Gerrit-Change-Number: 11461 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 18:41:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Hello Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11464 to look at the new patch set (#3). Change subject: IMPALA-7420: different error code for internal cancellation .. IMPALA-7420: different error code for internal cancellation I started by converting scan and spill-to-disk because the cancellation there is always meant to be internal to the scan and spill-to-disk subsystems. I updated all places that checked for TErrorCode::CANCELLED to treat CANCELLED_INTERNALLY the same. This is to aid triage and debugging of bugs like IMPALA-7418 where an "internal" cancellation leaks out into the query state. This will make it easier to determine if an internal cancellation somehow "leaked" out. Testing: Ran exhaustive tests. Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 --- M be/src/benchmarks/status-benchmark.cc M be/src/common/status.cc M be/src/common/status.h M be/src/exec/hdfs-orc-scanner.cc M be/src/exec/hdfs-parquet-scanner.cc M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scanner.cc M be/src/exec/scanner-context.cc M be/src/runtime/io/disk-io-mgr-test.cc 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/runtime-state.cc M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc M be/src/runtime/tmp-file-mgr.h M common/thrift/generate_error_codes.py 17 files changed, 86 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11464/3 -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. Patch Set 2: (1 comment) I found another place in TmpFileMgr that needed updating too. http://gerrit.cloudera.org:8080/#/c/11464/2/be/src/runtime/io/request-context.cc File be/src/runtime/io/request-context.cc: http://gerrit.cloudera.org:8080/#/c/11464/2/be/src/runtime/io/request-context.cc@256 PS2, Line 256: CANCELLED > nit: CONTEXT_CANCELLED Done -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 18:50:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 18:51:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3179/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 18:51:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. Patch Set 3: Code-Review+2 carry -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 18:50:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11470 ) Change subject: IMPALA-7306: regression test for non-removed transient updates .. Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@73 PS1, Line 73: 0 flake8: E501 line too long (92 > 90 characters) http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@301 PS1, Line 301: W flake8: E122 continuation line missing indentation or outdented http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@324 PS1, Line 324: t flake8: E122 continuation line missing indentation or outdented http://gerrit.cloudera.org:8080/#/c/11470/1/tests/statestore/test_statestore.py@780 PS1, Line 780: d flake8: E306 expected 1 blank line before a nested definition, found 0 -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 19 Sep 2018 18:51:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Hello Bharath Vissapragada, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11470 to look at the new patch set (#2). Change subject: IMPALA-7306: regression test for non-removed transient updates .. IMPALA-7306: regression test for non-removed transient updates Adds a test for IMPALA-7305 that reproduces the bug by delaying heartbeats and updates. Increased some timeouts in the test because they were hit once after looping for ~12 hours. Testing: Manually reintroduced the bug by commenting out the code that fixed it and confirmed that the test failed. Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc --- M tests/statestore/test_statestore.py 1 file changed, 61 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11470/2 -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11470 ) Change subject: IMPALA-7306: regression test for non-removed transient updates .. Patch Set 2: Fixed the bot comments -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 18:56:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11471 Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. IMPALA-7594: Fix the waiting time in test_automatic_invalidation IMPALA-7593 increased the table invalidation timeout but the time to wait for the effect should also be increased. Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 --- M tests/custom_cluster/test_automatic_invalidation.py 1 file changed, 5 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/11471/1 -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 12: (9 comments) http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/coordinator-backend-state.cc@497 PS12, Line 497: last_report_seq_no_ = exec_status.report_seq_no(); > Can you add a CHECK here that it isn't decreasing? It's verified above, but Done. Used a DCHECK instead for documenting the invariant. http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/coordinator-backend-state.cc@508 PS12, Line 508: lock_guard l1(exec_summary->lock); > is this critical section extending farther than it needs to? should it end Fixed. http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/fragment-instance-state.h File be/src/runtime/fragment-instance-state.h: http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/fragment-instance-state.h@128 PS12, Line 128: int32_t ReportSeqNo() { > nit: maybe rename to AdvanceReportSeqNo or NextReportSeqNo or something so Done http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/fragment-instance-state.h@178 PS12, Line 178: DFAKE_MUTEX(report_seq_no_lock_); > I think this fake mutex should be held around the whole function that gener Good point. Moved to SendReport() instead. http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/12/be/src/runtime/query-state.cc@306 PS12, Line 306: REPORT_EXEC_STATUS > hmm, I dont see this one used. did I miss something? Nice catch. I meant to rename this to REPORT_EXEC_STATUS_PROFILE. http://gerrit.cloudera.org:8080/#/c/10855/12/common/protobuf/CMakeLists.txt File common/protobuf/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/10855/12/common/protobuf/CMakeLists.txt@47 PS12, Line 47: KRPC_GENERATE(CONTROL_SVC_PROTO_SRCS CONTROL_SVC_PROTO_HDRS > You could factor out the replication of the protobuf generation by putting Good idea. Adopted something similar to your suggested code. http://gerrit.cloudera.org:8080/#/c/10855/12/common/protobuf/common.proto File common/protobuf/common.proto: http://gerrit.cloudera.org:8080/#/c/10855/12/common/protobuf/common.proto@33 PS12, Line 33: int64 > nit: i think fixed64 is more appropriate for these fields since they dont t Agreed that they don't tend to be small. Does using fixed64 make the encoding slightly faster ? http://gerrit.cloudera.org:8080/#/c/10855/12/tests/custom_cluster/test_rpc_timeout.py File tests/custom_cluster/test_rpc_timeout.py: http://gerrit.cloudera.org:8080/#/c/10855/12/tests/custom_cluster/test_rpc_timeout.py@42 PS12, Line 42: > flake8: E251 unexpected spaces around keyword / parameter equals Done http://gerrit.cloudera.org:8080/#/c/10855/12/tests/custom_cluster/test_rpc_timeout.py@42 PS12, Line 42: > flake8: E251 unexpected spaces around keyword / parameter equals Done -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 12 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 19 Sep 2018 19:01:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Hello Sailesh Mukil, Todd Lipcon, Impala Public Jenkins, Dan Hecht, Michal Ostrowski, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10855 to look at the new patch set (#13). Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC This change converts ReportExecStatus() RPC from thrift based RPC to KRPC. This is done in part of the preparation for fixing IMPALA-2990 as we can take advantage of TCP connection multiplexing in KRPC to avoid overwhelming the coordinator with too many connections by reducing the number of TCP connection to one for each executor. This patch also introduces a new service pool for all query execution control related RPCs in the future so that control commands from coordinators aren't blocked by long-running DataStream services' RPCs. The majority of this patch is mechanical conversion of some Thrift structures used in ReportExecStatus() RPC to Protobuf. Note that the runtime profile is still retained as a Thrift structure as Impala clients will still fetch query profiles using Thrift RPCs. This also avoids duplicating the serialization implementation in both Thrift and Protobuf for the runtime profile. The Thrift runtime profiles are serialized and sent as a sidecar in ReportExecStatus() RPC. This patch also fixes IMPALA-7241 which may lead to duplicated dml stats being applied. The fix is by adding a monotonically increasing version number for fragment instances' reports. The coordinator will ignore any report smaller than or equal to the version in the last report. Testing done: 1. Exhaustive build. 2. Added some targeted test cases for profile serialization failure and RPC retries/timeout. Change-Id: I7638583b433dcac066b87198e448743d90415ebe --- M be/src/benchmarks/expr-benchmark.cc M be/src/catalog/catalog-util.cc M be/src/common/global-flags.cc M be/src/exec/data-sink.cc M be/src/exec/data-sink.h M be/src/exec/hbase-table-sink.cc M be/src/exec/hdfs-parquet-table-writer.cc M be/src/exec/hdfs-parquet-table-writer.h M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-table-writer.cc M be/src/exec/hdfs-table-writer.h M be/src/rpc/CMakeLists.txt M be/src/rpc/jni-thrift-util.h M be/src/rpc/thrift-util-test.cc M be/src/rpc/thrift-util.h M be/src/runtime/backend-client.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/dml-exec-state.cc M be/src/runtime/dml-exec-state.h M be/src/runtime/exec-env.cc M be/src/runtime/exec-env.h M be/src/runtime/fragment-instance-state.cc M be/src/runtime/fragment-instance-state.h M be/src/runtime/query-state.cc M be/src/runtime/query-state.h M be/src/runtime/runtime-state.cc M be/src/runtime/runtime-state.h M be/src/runtime/test-env.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler-test-util.cc M be/src/service/CMakeLists.txt M be/src/service/client-request-state.cc M be/src/service/client-request-state.h A be/src/service/control-service.cc A be/src/service/control-service.h M be/src/service/data-stream-service.cc M be/src/service/data-stream-service.h M be/src/service/impala-internal-service.cc M be/src/service/impala-internal-service.h M be/src/service/impala-server.cc M be/src/service/impala-server.h M be/src/testutil/in-process-servers.cc M be/src/util/container-util.h A be/src/util/error-util-internal.h M be/src/util/error-util-test.cc M be/src/util/error-util.cc M be/src/util/error-util.h M be/src/util/runtime-profile.cc M be/src/util/uid-util.h M common/protobuf/CMakeLists.txt M common/protobuf/common.proto A common/protobuf/control_service.proto M common/protobuf/data_stream_service.proto M common/protobuf/row_batch.proto M common/protobuf/rpc_test.proto M common/thrift/ImpalaInternalService.thrift M tests/custom_cluster/test_rpc_timeout.py 60 files changed, 1,212 insertions(+), 758 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/10855/13 -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7498: Fix log spew from LocalCatalog startup
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11472 Change subject: IMPALA-7498: Fix log spew from LocalCatalog startup .. IMPALA-7498: Fix log spew from LocalCatalog startup Frontend calls LocalCatalog#waitForCatalog() in a tight loop during startup and ends up spewing tons of log messages if the MetaProvider takes some time to initialize. This is a problem for CatalogdMetaProvider implementation since it waits on the Catalog server to send an initial update. This can take some time depending on various factors and the logs become too noisy during that period. This patch adds a sleep() inside waitForCatalog() to avoid this. We can do something fancy, like ImpaladCatalog implementation, by waiting on a synchronized monitor and getting notified when the Catalog is initialized, but I don't think it is worth the effort, especially since the sleep time is pretty small (MAX_CATALOG_UPDATE_WAIT_TIME_MS = 2s). Testing: Tested it locally by just starting the impalad catalog in local mode and the logs look much better while waiting for the Catalog to send the initial update. Change-Id: I81e7cc6f464fd71afecd94a5a2b777aff47ee11b --- M fe/src/main/java/org/apache/impala/catalog/local/LocalCatalog.java 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/72/11472/1 -- To view, visit http://gerrit.cloudera.org:8080/11472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I81e7cc6f464fd71afecd94a5a2b777aff47ee11b Gerrit-Change-Number: 11472 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/711/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 19:39:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/713/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 19:45:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11470 ) Change subject: IMPALA-7306: regression test for non-removed transient updates .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/712/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 19:51:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7498: Fix log spew from LocalCatalog startup
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11472 ) Change subject: IMPALA-7498: Fix log spew from LocalCatalog startup .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/715/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I81e7cc6f464fd71afecd94a5a2b777aff47ee11b Gerrit-Change-Number: 11472 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 19 Sep 2018 20:01:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py@35 PS1, Line 35: timeout = 20 if IMPALAD_BUILD.runs_slowly() or (not IS_HDFS and not IS_LOCAL) else 10 Basic question, for my understanding. Why does this param depend on the build type? If it is a slow build like ASAN, it takes time for the coordinators to report the usage metrics or something else? Trying to understand where the delay is. http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py@53 PS1, Line 53: self.timeout * 2 qq, 2*timeout because we want to make sure it is invalidated timeout before the next cycle of invalidation kicks in or is it just empirical? -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 20:02:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/714/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 19 Sep 2018 20:05:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py@35 PS1, Line 35: timeout = 20 if IMPALAD_BUILD.runs_slowly() or (not IS_HDFS and not IS_LOCAL) else 10 > Basic question, for my understanding. Why does this param depend on the bui The tests will run a query and assumes the table used in the query is loaded after the query finishes. If the query runs slowly the table might have been invalidated when the query finishes. http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py@53 PS1, Line 53: self.timeout * 2 > qq, 2*timeout because we want to make sure it is invalidated timeout before It only needs to be a little larger than timeout. 2* is empirical -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 20:05:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7584: set.test: omit pool name in catch string
David Knupp has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11476 Change subject: IMPALA-7584: set.test: omit pool name in catch string .. IMPALA-7584: set.test: omit pool name in catch string Running this test on an actual cluster results in a failure, since the pool name differs. Removing the name allows this test to pass on clusters. Also tested that minicluster tests still pass. Change-Id: I1529c040520a1d8e7ca47960c76028b2579c8d03 --- M testdata/workloads/functional-query/queries/QueryTest/set.test 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/11476/1 -- To view, visit http://gerrit.cloudera.org:8080/11476 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1529c040520a1d8e7ca47960c76028b2579c8d03 Gerrit-Change-Number: 11476 Gerrit-PatchSet: 1 Gerrit-Owner: David Knupp
[Impala-ASF-CR] IMPALA-7585: Always set user credentials after creating RPC proxy
Michael Ho has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11477 Change subject: IMPALA-7585: Always set user credentials after creating RPC proxy .. IMPALA-7585: Always set user credentials after creating RPC proxy kudu::rpc::Proxy() ctor may fail in GetLoggedInUser() for various reasons (e.g. missing certain libraries). This resulted in an empty username being used in kudu::rpc::ConnectionId. With plaintext SASL (e.g. in an insecure Impala cluster), this may result in the following error during connection negotiation: Not authorized: Client connection negotiation failed: client connection to 127.0.0.1:27000: SASL(-1): generic failure: All-whitespace username. In Impala, we don't consider failing GetLoggedInUser() a fatal error. This change fixes the issue above by always explicitly setting the username after creating the proxy. The username is "impala". Please note that this username is not really used anywhere for authorization for RPC services. Authorization is only done when authentication is enabled with Kerberos. With Kerberos enabled, the username is derived from the Kerberos principal instead of the user credentials set in the ConnectionId. It's there mostly to satisfy the SASL plaintext case. rpc-mgr-test has been updated to test for this string when Kerberos is disabled. Testing done: core test; rpc-mgr-test; rpc-mgr-kerberized-test Change-Id: I75059f55bcdb8f95916610100cad4d8280daf3f6 --- M be/src/exec/kudu-util.h M be/src/rpc/rpc-mgr-test.h M be/src/rpc/rpc-mgr.inline.h 3 files changed, 16 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/11477/1 -- To view, visit http://gerrit.cloudera.org:8080/11477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I75059f55bcdb8f95916610100cad4d8280daf3f6 Gerrit-Change-Number: 11477 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3177/ -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 20:29:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py File tests/custom_cluster/test_automatic_invalidation.py: http://gerrit.cloudera.org:8080/#/c/11471/1/tests/custom_cluster/test_automatic_invalidation.py@35 PS1, Line 35: timeout = 20 if IMPALAD_BUILD.runs_slowly() or (not IS_HDFS and not IS_LOCAL) else 10 > The tests will run a query and assumes the table used in the query is loade Oh I see, can you added that comment here. It is not super obvious -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 20:40:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7584: set.test: omit pool name in catch string
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11476 ) Change subject: IMPALA-7584: set.test: omit pool name in catch string .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/716/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11476 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1529c040520a1d8e7ca47960c76028b2579c8d03 Gerrit-Change-Number: 11476 Gerrit-PatchSet: 1 Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 20:44:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7585: Always set user credentials after creating RPC proxy
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11477 ) Change subject: IMPALA-7585: Always set user credentials after creating RPC proxy .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/717/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11477 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I75059f55bcdb8f95916610100cad4d8280daf3f6 Gerrit-Change-Number: 11477 Gerrit-PatchSet: 1 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Wed, 19 Sep 2018 20:50:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11470 ) Change subject: IMPALA-7306: regression test for non-removed transient updates .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/11470/2/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/11470/2/tests/statestore/test_statestore.py@302 PS2, Line 302: W flake8: E122 continuation line missing indentation or outdented http://gerrit.cloudera.org:8080/#/c/11470/2/tests/statestore/test_statestore.py@325 PS2, Line 325: t flake8: E122 continuation line missing indentation or outdented -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 21:02:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7584: set.test: omit pool name in catch string
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11476 ) Change subject: IMPALA-7584: set.test: omit pool name in catch string .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11476 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1529c040520a1d8e7ca47960c76028b2579c8d03 Gerrit-Change-Number: 11476 Gerrit-PatchSet: 1 Gerrit-Owner: David Knupp Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 21:05:29 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 3: Failed because of another issue, I'll file a bug for it and rerun the merge -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 21:12:24 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Tim Armstrong has removed a vote on this change. Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Removed Verified-1 by Impala Public Jenkins -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3180/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 21:12:36 + Gerrit-HasComments: No
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 3: Filed I MPALA-7595 -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 21:43:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Hello Bharath Vissapragada, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11470 to look at the new patch set (#3). Change subject: IMPALA-7306: regression test for non-removed transient updates .. IMPALA-7306: regression test for non-removed transient updates Adds a test for IMPALA-7305 that reproduces the bug by delaying heartbeats and updates. Increased some timeouts in the test because they were hit once after looping for ~12 hours. Testing: Manually reintroduced the bug by commenting out the code that fixed it and confirmed that the test failed. Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc --- M tests/statestore/test_statestore.py 1 file changed, 61 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/11470/3 -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-6772: Build ORC lib without libhdfspp and apply ORC-403
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11442 ) Change subject: IMPALA-6772: Build ORC lib without libhdfspp and apply ORC-403 .. Patch Set 2: (1 comment) That is awesome news about the fuzz tests. I had one question about the patch but otherwise looks great. http://gerrit.cloudera.org:8080/#/c/11442/2/source/orc/orc-1.5.2-patches/0002-ORC-403-C-Add-checks-to-avoid-invalid-offsets-in-Inp.patch File source/orc/orc-1.5.2-patches/0002-ORC-403-C-Add-checks-to-avoid-invalid-offsets-in-Inp.patch: http://gerrit.cloudera.org:8080/#/c/11442/2/source/orc/orc-1.5.2-patches/0002-ORC-403-C-Add-checks-to-avoid-invalid-offsets-in-Inp.patch@20 PS2, Line 20: +++ b/c++/src/Reader.cc I compared to the upstream patch for ORC-403 and this seems to be missing some of the changes for Reader.cc at lines 889 and 997. Were those deliberately removed? -- To view, visit http://gerrit.cloudera.org:8080/11442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1491b558a7972c0f77b0bfeb6e9cc04fda1734e9 Gerrit-Change-Number: 11442 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 21:53:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7335: Fix a race in HdfsScanNode
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11337 ) Change subject: IMPALA-7335: Fix a race in HdfsScanNode .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/11337 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id470818846a5c69ad28a6ff695069702982aa793 Gerrit-Change-Number: 11337 Gerrit-PatchSet: 7 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 21:59:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7579: fix test query profile contains all events on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11461 ) Change subject: IMPALA-7579: fix test_query_profile_contains_all_events on S3 .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11461 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7c6e0899455dd4e12636b959fab4bde79f02fb95 Gerrit-Change-Number: 11461 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 22:07:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7579: fix test query profile contains all events on S3
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11461 ) Change subject: IMPALA-7579: fix test_query_profile_contains_all_events on S3 .. IMPALA-7579: fix test_query_profile_contains_all_events on S3 This bug was introduced by IMPALA-6568 which added the test_query_profile_contains_all_events test. This test creates a file in the filesystem so that it can be used by 'load data inpath'. The test was using the hdfs_client object to do file system operations, but this client only works with hdfs. Switch to using the filesystem_client object which can work on other filesystems, including s3. Also create the file that will be moved by 'load data inpath' under the unique database directory; this means the file can be created without having to check if it exists already and must be deleted. Change-Id: I7c6e0899455dd4e12636b959fab4bde79f02fb95 Reviewed-on: http://gerrit.cloudera.org:8080/11461 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/query_test/test_observability.py 1 file changed, 2 insertions(+), 3 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11461 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7c6e0899455dd4e12636b959fab4bde79f02fb95 Gerrit-Change-Number: 11461 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. IMPALA-7420: different error code for internal cancellation I started by converting scan and spill-to-disk because the cancellation there is always meant to be internal to the scan and spill-to-disk subsystems. I updated all places that checked for TErrorCode::CANCELLED to treat CANCELLED_INTERNALLY the same. This is to aid triage and debugging of bugs like IMPALA-7418 where an "internal" cancellation leaks out into the query state. This will make it easier to determine if an internal cancellation somehow "leaked" out. Testing: Ran exhaustive tests. Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Reviewed-on: http://gerrit.cloudera.org:8080/11464 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/benchmarks/status-benchmark.cc M be/src/common/status.cc M be/src/common/status.h M be/src/exec/hdfs-orc-scanner.cc M be/src/exec/hdfs-parquet-scanner.cc M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scanner.cc M be/src/exec/scanner-context.cc M be/src/runtime/io/disk-io-mgr-test.cc 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/runtime-state.cc M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc M be/src/runtime/tmp-file-mgr.h M common/thrift/generate_error_codes.py 17 files changed, 86 insertions(+), 67 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7420: different error code for internal cancellation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11464 ) Change subject: IMPALA-7420: different error code for internal cancellation .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11464 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If25d5b539d68981359e4d881cae7b08728ba2999 Gerrit-Change-Number: 11464 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 22:19:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7306: regression test for non-removed transient updates
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11470 ) Change subject: IMPALA-7306: regression test for non-removed transient updates .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/718/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11470 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c2a39d8a76cb5371f394b5a97817d8231e473cc Gerrit-Change-Number: 11470 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 22:20:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-110: Support for multiple DISTINCT
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10771 ) Change subject: IMPALA-110: Support for multiple DISTINCT .. Patch Set 12: Are we waiting for more reviews here? I haven't double-checked the backend changes, which I'd want to do before finally +2ing but if we think that's converged I can take a look. -- To view, visit http://gerrit.cloudera.org:8080/10771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I055402eaef6d81e5f70e850d9f8a621e766830a4 Gerrit-Change-Number: 10771 Gerrit-PatchSet: 12 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 19 Sep 2018 22:20:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7556: part 1: handle different file systems via polymorphism
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11444 ) Change subject: IMPALA-7556: part 1: handle different file systems via polymorphism .. Patch Set 2: (12 comments) http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/file-reader.h File be/src/runtime/io/file-reader.h: http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/file-reader.h@78 PS3, Line 78: // Debug string of this file reader. Optional: switch to SpinLock to reduce overhead on uncontented acquisition path. http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/file-reader.h File be/src/runtime/io/file-reader.h: http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/file-reader.h@93 PS2, Line 93: ScanRange* scan_range_; > I changed it to a reference. I was thinking ScanRange* const scan_range_;. Using a reference is a bit inconsistent with some other parts of the code but I think captures the intent. http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.h File be/src/runtime/io/hdfs-file-reader.h: http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.h@51 PS3, Line 51: /// 2. The scan range is using hdfs caching. I should have been clearer - I meant "hdfsFS const hdfs_fs_" since we don't expect the pointer to be reassigned. It's good we can make the pointed-to memory const too. Ok to ignore, this is fine. http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.h@78 PS3, Line 78: Maybe "non-NULL"? http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc File be/src/runtime/io/hdfs-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/11444/2/be/src/runtime/io/hdfs-file-reader.cc@108 PS2, Line 108: int64_t max_chunk_size = scan_range_->MaxReadChunkSize(); > E.g. should I annotate PS1 with gerrit comments? It is the closest PS to th Yeah if there's a large mechanical component to the changes you made I find it helpful to understand what you did so I can focus on checking the correctness of the mechanical transformation instead of re-reviewing existing code. There's some subtle edge cases in this code so I was mainly wanting to make sure that we preserved them (we should have test cases for them all but I wouldn't bet my life on it). Thomas did something nice with https://gerrit.cloudera.org/#/c/10394/, although that patch was way more unwieldy than this. I sometimes review these things by manually putting the before and after moved code and diffing the two files (although I suspect there are some tools to better automate that that I don't know about yet). I did that for this and I'm basically content now. http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.cc File be/src/runtime/io/hdfs-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.cc@171 PS3, Line 171: io_mgr_->ReopenCachedHdfsFileHandle(hdfs_fs_, scan_range_->file_string(), Same comment about local-file-reader for simplifying *eosr logic. http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.cc@194 PS3, Line 194: return status; I think we can just return the status here, no? We don't need the seek_failed stuff post-refactor. http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.cc@204 PS3, Line 204: if (exclusive_hdfs_fh_ != nullptr) { We can just return here I think http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/hdfs-file-reader.cc@210 PS3, Line 210: scan_range_->external_buffer_tag_ = ScanRange::ExternalBufferTag::NO_BUFFER; If we return early on the error paths, this just becomes return Status::OK(). http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/local-file-reader.cc File be/src/runtime/io/local-file-reader.cc: http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/local-file-reader.cc@87 PS3, Line 87: } else { We returned on the first branch so we can avoid nested the else branch. http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/local-file-reader.cc@95 PS3, Line 95: if (bytes_read_ == scan_range_->len()) Conditional needs to either be on one line or have parentheses. Or we could actually just rewrite as *eosr = bytes_read_ == scan_range_.len() and avoid initialising above. http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/request-ranges.h File be/src/runtime/io/request-ranges.h: http://gerrit.cloudera.org:8080/#/c/11444/3/be/src/runtime/io/request-ranges.h@339 PS3, Line 339: /// END: private members that are accessed by other io:: classes I also modified this comment in https://gerrit.cloudera.org/#/c/11464/, sorry for the rebase conflict in advance. -- To view, visit http://gerrit.cloudera.org:8080/11444 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-
[Impala-ASF-CR] IMPALA-7335: Fix a race in HdfsScanNode
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11337 ) Change subject: IMPALA-7335: Fix a race in HdfsScanNode .. Patch Set 7: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/11337/6/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11337/6/be/src/exec/hdfs-scan-node.cc@538 PS6, Line 538: unique_lock l(lock_); > Done. since this is only a helper functions to call SetDoneInternal you can move comments to the method comment in the header file. How about "Gets lock_ and calls SetDoneInternal(status_). Usually used after the scan node completes execution successfully." -- To view, visit http://gerrit.cloudera.org:8080/11337 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id470818846a5c69ad28a6ff695069702982aa793 Gerrit-Change-Number: 11337 Gerrit-PatchSet: 7 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 22:45:41 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 3 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 22:47:07 + Gerrit-HasComments: No
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11478 Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". This instructs git to pretend that certain "minimized" and debugging Bootstrap-related files are treated as binary by git tools, especially "git grep". As a result, doing something like "git grep pause" won't yield a wall of indecipherable JavaScript. Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 --- A .gitattributes 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/11478/1 -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Tianyi Wang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. IMPALA-7594: Fix the waiting time in test_automatic_invalidation IMPALA-7593 increased the table invalidation timeout but the time to wait for the effect should also be increased. Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 --- M tests/custom_cluster/test_automatic_invalidation.py 1 file changed, 7 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/11471/2 -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics.
Hello Bharath Vissapragada, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11468 to look at the new patch set (#2). Change subject: IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. .. IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. Following up to IMPALA-6857, it's useful for monitoring tools to see if the pause monitor is getting triggered, and to see other GC metrics. The Java side here, and the Thrift side, were easy enough. However, the Impala metric implementation here caused us to call into the frontend to read through the JMX memory beans 72 times, because each call to GetValue() was getting all the data for the pool. This structure made it hard to add additional, non-pool, metrics, and it felt wasteful. To combat this, I added a cache of 10 seconds for getting the metrics from the Frontend. The counters will typically re-use the same data. There are five metrics here, and to avoid yet another enum class, I used C++ lambdas to capture which field of the Thrift object I care about. If folks like the approach, I think it can simplify way the enums for the pool metrics as well. I measured the cost of calling into the metrics code by looping the metrics-gathering 100 times and looking at CPU time for the process using this script: START_CPU=$(cat /proc/$(fuser 25000/tcp 2> /dev/null | tr -d ' ')/stat | awk '{ print $14 + $15 }') for i in $(seq 100); do curl http://localhost:25000/jsonmetrics?json > /dev/null 2> /dev/null done END_CPU=$( cat /proc/$(fuser 25000/tcp 2> /dev/null | tr -d ' ')/stat | awk '{ print $14 + $15 }') echo $START_CPU $END_CPU $(($END_CPU - $START_CPU)) On a release build on my development machine, gathering metrics 100 times took 0.16 cpu seconds without this change and 0.07 cpu seconds with this change. The measurement accuracy here is 0.01 (I spot-checked this with using the cpuacct cgroup infrastructure which gives you nanos, but it was more painful to script), but this convinces me that this is a net improvement. TODO: Update tests to check new metrics. Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 --- M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M common/thrift/Frontend.thrift M common/thrift/metrics.json M fe/src/main/java/org/apache/impala/common/JniUtil.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java M tests/custom_cluster/test_pause_monitor.py A tests/infra/test_jvm_metrics.py 8 files changed, 352 insertions(+), 128 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/11468/2 -- To view, visit http://gerrit.cloudera.org:8080/11468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 Gerrit-Change-Number: 11468 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3181/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 3 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 22:47:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics.
Hello Bharath Vissapragada, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11468 to look at the new patch set (#3). Change subject: IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. .. IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. Following up to IMPALA-6857, it's useful for monitoring tools to see if the pause monitor is getting triggered, and to see other GC metrics. The Java side here, and the Thrift side, were easy enough. However, the Impala metric implementation here caused us to call into the frontend to read through the JMX memory beans 72 times, because each call to GetValue() was getting all the data for the pool. This structure made it hard to add additional, non-pool, metrics, and it felt wasteful. To combat this, I added a cache of 10 seconds for getting the metrics from the Frontend. The counters will typically re-use the same data. There are five metrics here, and to avoid yet another enum class, I used C++ lambdas to capture which field of the Thrift object I care about. If folks like the approach, I think it can simplify way the enums for the pool metrics as well. I measured the cost of calling into the metrics code by looping the metrics-gathering 100 times and looking at CPU time for the process using this script: START_CPU=$(cat /proc/$(fuser 25000/tcp 2> /dev/null | tr -d ' ')/stat | awk '{ print $14 + $15 }') for i in $(seq 100); do curl http://localhost:25000/jsonmetrics?json > /dev/null 2> /dev/null done END_CPU=$( cat /proc/$(fuser 25000/tcp 2> /dev/null | tr -d ' ')/stat | awk '{ print $14 + $15 }') echo $START_CPU $END_CPU $(($END_CPU - $START_CPU)) On a release build on my development machine, gathering metrics 100 times took 0.16 cpu seconds without this change and 0.07 cpu seconds with this change. The measurement accuracy here is 0.01 (I spot-checked this with using the cpuacct cgroup infrastructure which gives you nanos, but it was more painful to script), but this convinces me that this is a net improvement. TODO: Update tests to check new metrics. Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 --- M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M common/thrift/Frontend.thrift M common/thrift/metrics.json M fe/src/main/java/org/apache/impala/common/JniUtil.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java M tests/custom_cluster/test_pause_monitor.py A tests/infra/test_jvm_metrics.py 8 files changed, 352 insertions(+), 128 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/11468/3 -- To view, visit http://gerrit.cloudera.org:8080/11468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 Gerrit-Change-Number: 11468 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-589: Add a sql function to return the impalad coordinator hostname for diagnostic purposes.
Zoram Thanga has posted comments on this change. ( http://gerrit.cloudera.org:8080/11459 ) Change subject: IMPALA-589: Add a sql function to return the impalad coordinator hostname for diagnostic purposes. .. Patch Set 1: Code-Review+1 (1 comment) Change looks good to me. It would be great to have the function take an optional query id argument so we can use it on a running query as well. But I realize that that's a lot of work, and the same info is discoverable from the CM. http://gerrit.cloudera.org:8080/#/c/11459/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/11459/1/be/src/exprs/expr-test.cc@4961 PS1, Line 4961: ObjectPool pool; This does not seem to be used. -- To view, visit http://gerrit.cloudera.org:8080/11459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94d6e2664ba659b48df53c5c06f67b502c533e47 Gerrit-Change-Number: 11459 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 19 Sep 2018 23:06:04 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 1: There's also some minified JS under ./www: ./www/dagre-d3.min.js ./www/bootstrap/js/bootstrap.min.js ./www/datatables.min.js ./www/jquery/jquery-1.12.4.min.js ./www/d3.v3.min.js ./www/DataTables-1.10.12/js/dataTables.jqueryui.min.js ./www/DataTables-1.10.12/js/jquery.dataTables.min.js ./www/DataTables-1.10.12/js/dataTables.foundation.min.js ./www/DataTables-1.10.12/js/dataTables.bootstrap.min.js -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:09:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10855 ) Change subject: IMPALA-7213, IMPALA-7241: Port ReportExecStatus() RPC to use KRPC .. Patch Set 13: (2 comments) http://gerrit.cloudera.org:8080/#/c/10855/13/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/13/be/src/runtime/coordinator-backend-state.cc@523 PS13, Line 523: << " node_id=" << node_id << " instance_id=" << PrintId(exec_params_.instance_id) line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/10855/13/be/src/runtime/coordinator-backend-state.cc@530 PS13, Line 530: if (rows_counter != nullptr) instance_stats.__set_cardinality(rows_counter->value()); line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/10855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7638583b433dcac066b87198e448743d90415ebe Gerrit-Change-Number: 10855 Gerrit-PatchSet: 13 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Michal Ostrowski Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 19 Sep 2018 23:15:24 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/719/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 19 Sep 2018 23:15:43 + Gerrit-HasComments: No
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11478 to look at the new patch set (#2). Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". This instructs git to pretend that certain "minimized" and debugging Bootstrap-related files are treated as binary by git tools, especially "git grep". As a result, doing something like "git grep pause" won't yield a wall of indecipherable JavaScript. Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 --- A .gitattributes 1 file changed, 20 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/11478/2 -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:17:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7335: Fix a race in HdfsScanNode
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11337 ) Change subject: IMPALA-7335: Fix a race in HdfsScanNode .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/11337/6/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11337/6/be/src/exec/hdfs-scan-node.cc@538 PS6, Line 538: unique_lock l(lock_); > since this is only a helper functions to call SetDoneInternal you can move Done -- To view, visit http://gerrit.cloudera.org:8080/11337 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id470818846a5c69ad28a6ff695069702982aa793 Gerrit-Change-Number: 11337 Gerrit-PatchSet: 8 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:24:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7335: Fix a race in HdfsScanNode
Pooja Nilangekar has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/11337 ) Change subject: IMPALA-7335: Fix a race in HdfsScanNode .. IMPALA-7335: Fix a race in HdfsScanNode This change fixes the race between the done_ flag and the status_ variable in the HdfsScanNode. Previously, a scanner thread would mark its scan range as complete even if it ran into an error. Another scanner thread could notice that all scan ranges have completed and set the done_ flag before the status_ variable is updated with the non-ok status. This caused the main thread to return an ok status despite the scanner thread running into an error. This change ensures that when a scanner thread runs into an error, it updates the status_ before marking its scan range as complete. Testing: Ran core tests. Since there is no deterministic method to reproduce the race, this change does not include any regression tests. Additionally, this change also fixes IMPALA-7430 by removing the logs added to debug this race. Change-Id: Id470818846a5c69ad28a6ff695069702982aa793 --- M be/src/exec/hdfs-scan-node.cc M be/src/exec/hdfs-scan-node.h 2 files changed, 40 insertions(+), 40 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/11337/8 -- To view, visit http://gerrit.cloudera.org:8080/11337 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id470818846a5c69ad28a6ff695069702982aa793 Gerrit-Change-Number: 11337 Gerrit-PatchSet: 8 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:27:25 + Gerrit-HasComments: No
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3182/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:27:27 + Gerrit-HasComments: No
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 1: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/721/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:30:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11468 ) Change subject: IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. .. Patch Set 3: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/722/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 Gerrit-Change-Number: 11468 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:40:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11468 ) Change subject: IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/720/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 Gerrit-Change-Number: 11468 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:39:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-2063 Remove newline characters in query status.
Hello Michael Ho, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11425 to look at the new patch set (#4). Change subject: IMPALA-2063 Remove newline characters in query status. .. IMPALA-2063 Remove newline characters in query status. Remove extraneous whitespace at the end of strings being added to profiles. Remove any duplicated newline characters within strings as well. (The latter step is necessary to allow for a blanket assertion on this in testing.) Change-Id: I2bbd7d7fe2c6d0f3799d0e6b336710bccfef0ab1 --- M be/src/util/runtime-profile.cc M be/src/util/runtime-profile.h M tests/query_test/test_cancellation.py 3 files changed, 33 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/11425/4 -- To view, visit http://gerrit.cloudera.org:8080/11425 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2bbd7d7fe2c6d0f3799d0e6b336710bccfef0ab1 Gerrit-Change-Number: 11425 Gerrit-PatchSet: 4 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/723/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 2 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:48:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7335: Fix a race in HdfsScanNode
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11337 ) Change subject: IMPALA-7335: Fix a race in HdfsScanNode .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/724/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11337 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id470818846a5c69ad28a6ff695069702982aa793 Gerrit-Change-Number: 11337 Gerrit-PatchSet: 8 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 19 Sep 2018 23:58:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics.
Hello Bharath Vissapragada, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11468 to look at the new patch set (#4). Change subject: IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. .. IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. Following up to IMPALA-6857, it's useful for monitoring tools to see if the pause monitor is getting triggered, and to see other GC metrics. The Java side here, and the Thrift side, were easy enough. However, the Impala metric implementation here caused us to call into the frontend to read through the JMX memory beans 72 times, because each call to GetValue() was getting all the data for the pool. This structure made it hard to add additional, non-pool, metrics, and it felt wasteful. To combat this, I added a cache of 10 seconds for getting the metrics from the Frontend. The counters will typically re-use the same data. There are five metrics here, and to avoid yet another enum class, I used C++ lambdas to capture which field of the Thrift object I care about. If folks like the approach, I think it can simplify way the enums for the pool metrics as well. I measured the cost of calling into the metrics code by looping the metrics-gathering 100 times and looking at CPU time for the process using this script: START_CPU=$(cat /proc/$(fuser 25000/tcp 2> /dev/null | tr -d ' ')/stat | awk '{ print $14 + $15 }') for i in $(seq 100); do curl http://localhost:25000/jsonmetrics?json > /dev/null 2> /dev/null done END_CPU=$( cat /proc/$(fuser 25000/tcp 2> /dev/null | tr -d ' ')/stat | awk '{ print $14 + $15 }') echo $START_CPU $END_CPU $(($END_CPU - $START_CPU)) On a release build on my development machine, gathering metrics 100 times took 0.16 cpu seconds without this change and 0.07 cpu seconds with this change. The measurement accuracy here is 0.01 (I spot-checked this with using the cpuacct cgroup infrastructure which gives you nanos, but it was more painful to script), but this convinces me that this is a net improvement. TODO: Update tests to check new metrics. Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 --- M be/src/util/memory-metrics.cc M be/src/util/memory-metrics.h M common/thrift/Frontend.thrift M common/thrift/metrics.json M fe/src/main/java/org/apache/impala/common/JniUtil.java M fe/src/main/java/org/apache/impala/util/JvmPauseMonitor.java M tests/custom_cluster/test_pause_monitor.py A tests/infra/test_jvm_metrics.py 8 files changed, 352 insertions(+), 127 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/11468/4 -- To view, visit http://gerrit.cloudera.org:8080/11468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 Gerrit-Change-Number: 11468 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-2063 Remove newline characters in query status.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11425 ) Change subject: IMPALA-2063 Remove newline characters in query status. .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/725/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11425 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2bbd7d7fe2c6d0f3799d0e6b336710bccfef0ab1 Gerrit-Change-Number: 11425 Gerrit-PatchSet: 4 Gerrit-Owner: Michal Ostrowski Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Comment-Date: Thu, 20 Sep 2018 00:12:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11468 ) Change subject: IMPALA-7596. Adding JvmPauseMonitor (and other GC) metrics to Impala metrics. .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/726/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia707393962ad94ef715ec015b3fe3bb1769104a2 Gerrit-Change-Number: 11468 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Sep 2018 00:36:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-589: Add sql function returning the impalad coordinator hostname.
Andrew Sherman has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11459 ) Change subject: IMPALA-589: Add sql function returning the impalad coordinator hostname. .. IMPALA-589: Add sql function returning the impalad coordinator hostname. In every execution of an Impala query, one of the impalad daemons acts as the coordinator node. In some cases, such as when using a proxy, a user cannot predict which host will act as the coordinator. To aid in diagnosis, we provide a sql function which returns the name of the host on which the coordinator is running. EXTERNAL DESCRIPTION: Add a builtin function called coordinator(), which returns the name of the host which is running the impalad that is acting as the coordinator for the current query. IMPLEMENTATION: All functions are passed a FunctionContext object which is the interface to the rest of the system for a UDF. From the FunctionContext we get the TQueryCtx (query context) which contains a TNetworkAddress for the coordinator. The hostname of the coordinator is copied from the TNetworkAddress. Can the TNetworkAddress for the coordinator be unitialized? In the thrift source the coord_address is marked optional, but my reading of the code says this is always set in a real impalad. To future-proof the code a null StringVal is returned if coord_address is unset. TESTING: - Added a basic unit test for the new function. - Added a unit test which simulates the case when coord_address is unset. - Hand tested in a development deployment. - Ran regression tests and got a clean run. LIMITATIONS: This change only adds the coordinator() function. It does not add the debug_url() function that was mentioned in the comments on the original Jira. Change-Id: I94d6e2664ba659b48df53c5c06f67b502c533e47 --- M be/src/exprs/expr-test.cc M be/src/exprs/utility-functions-ir.cc M be/src/exprs/utility-functions.h M common/function-registry/impala_functions.py M docs/topics/impala_misc_functions.xml 5 files changed, 60 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/11459/2 -- To view, visit http://gerrit.cloudera.org:8080/11459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I94d6e2664ba659b48df53c5c06f67b502c533e47 Gerrit-Change-Number: 11459 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-589: Add sql function returning the impalad coordinator hostname.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11459 ) Change subject: IMPALA-589: Add sql function returning the impalad coordinator hostname. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11459/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/11459/1/be/src/exprs/expr-test.cc@4961 PS1, Line 4961: ObjectPool pool; > This does not seem to be used. Thanks -- To view, visit http://gerrit.cloudera.org:8080/11459 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94d6e2664ba659b48df53c5c06f67b502c533e47 Gerrit-Change-Number: 11459 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 20 Sep 2018 00:38:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" The problem was caused by update in Hive with changed notifications. HIVE-15180 was added but was incomplete and resulted in the break. HIVE-17747 fixed the issue by properly creating the messages. Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Reviewed-on: http://gerrit.cloudera.org:8080/11466 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/create-test-configuration.sh M bin/impala-config.sh M common/thrift/JniCatalog.thrift M fe/src/main/java/org/apache/impala/analysis/AlterDbSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewSetOwnerStmt.java M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/Analyzer.java M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/util/SentryPolicyService.java M fe/src/main/java/org/apache/impala/util/SentryProxy.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java M fe/src/test/resources/mysql-hive-site.xml.template M fe/src/test/resources/postgresql-hive-site.xml.template M fe/src/test/resources/sentry-site.xml.template A fe/src/test/resources/sentry-site_no_oo.xml.template A fe/src/test/resources/sentry-site_oo.xml.template A fe/src/test/resources/sentry-site_oo_nogrant.xml.template M testdata/bin/run-sentry-service.sh M tests/authorization/test_grant_revoke.py A tests/authorization/test_owner_privileges.py M tests/common/custom_cluster_test_suite.py M tests/common/impala_test_suite.py 34 files changed, 1,365 insertions(+), 154 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 4 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER""
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11466 ) Change subject: Revert "Revert "IMPALA-7074: Update OWNER privilege on CREATE, DROP, and SET OWNER"" .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11466 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b9276c36bf96afccd7b8ff48803a30b47062c3d Gerrit-Change-Number: 11466 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Sep 2018 00:51:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-376: add built-in functions for parsing JSON
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10950 ) Change subject: IMPALA-376: add built-in functions for parsing JSON .. Patch Set 14: (23 comments) Based on the list discussion it sounds like we want to move forward with this. http://gerrit.cloudera.org:8080/#/c/10950/14//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10950/14//COMMIT_MSG@27 PS14, Line 27: * Add e2e tests in exprs.test Can we add a couple of tests to tests/custom_cluster/test_alloc_fail.py to check handling of out of memory. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/expr-test.cc@8856 PS14, Line 8856: TestIsNull("get_json_object('{illegal_json}', '$.a')", TYPE_STRING); Can you add some tests for: * trying to extract from just a JSON literal. E.g. numbers or strings * trying to extract from an empty list or object http://gerrit.cloudera.org:8080/#/c/10950/13/be/src/exprs/string-functions.cc File be/src/exprs/string-functions.cc: http://gerrit.cloudera.org:8080/#/c/10950/13/be/src/exprs/string-functions.cc@57 PS13, Line 57: void* Realloc(void* originalPtr, size_t originalSize, size_t newSize) { I think we need to copy over the previous contents of 'originalPtr'. I looked at the RapidJSON Realloc() implementations here and they both seem to do that: https://github.com/Tencent/rapidjson/blob/7e68aa0a21b7800ec98133cb106e49bd6536e25c/include/rapidjson/allocators.h http://gerrit.cloudera.org:8080/#/c/10950/13/be/src/exprs/string-functions.cc@133 PS13, Line 133: strlen Can we use sb.GetSize() instead of strlen()? It seems unnecessary to walk the string to find the null terminator. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc File be/src/exprs/string-functions.cc: http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@109 PS14, Line 109: StringVal ToStringVal(FunctionContext* ctx, JsonUdfAllocator* allocator, These functions can all be static I think. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@121 PS14, Line 121: memcpy(res.ptr, v->GetString(), v->GetStringLength()); Use StringVal::CopyFrom() instead - it handles the edge case when memory allocation fails. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@136 PS14, Line 136: return result; Use StringVal::CopyFrom() instead http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@138 PS14, Line 138: SelectByKey, SelectByIndex, ExpandArrays and ProcessWildcard key could benefit from short function comments explaining how they transform 'queue'. E.g. for SelectByKey something like: // Extract all the values for 'key' where objects in 'queue' contain that queue. // Replace the contents of queue with the values found. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@139 PS14, Line 139: (vector& queue, Our style says use pass by pointer for mutable arguments, and (generally) to put mutable arguments last: https://google.github.io/styleguide/cppguide.html#Reference_Arguments Exception: we sometimes put mutable arguments before immutable for context data structures like "FunctionContext*" http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@142 PS14, Line 142: for (int k = 0; k < old_items; ++k) { Why not use i for the index? here and below. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@162 PS14, Line 162: for (JsonUdfValue* it = queue[k]->Begin(); it != queue[k]->End(); ++it) Use braces here http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@172 PS14, Line 172: for (auto it = queue[k]->MemberBegin(); it != queue[k]->MemberEnd(); ++it) Use braces here http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@187 PS14, Line 187: Encounter nit: "Encountered" . Here and in a couple of other places like line 205. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@219 PS14, Line 219: return ++path_idx; // path_idx points at ']' Can't this be "return path_idx + 1". We don't use path_idx after returning. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@224 PS14, Line 224: int ProcessNumberIndex(FunctionContext* ctx, vector& queue, This is future work, but it would be nice in future to separate out the parsing of the path from the processing of the JSON. That way for constant path strings we could save re-parsing the path. The benefit probably isn't that huge but might be significant. http://gerrit.cloudera.org:8080/#/c/10950/14/be/src/exprs/string-functions.cc@275 PS14, Line 275: if (UNLIKELY(json_str.is_null || json_str.len == 0)) { nit: could fit on one
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 3 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 20 Sep 2018 02:12:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7594: Fix the waiting time in test automatic invalidation
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11471 ) Change subject: IMPALA-7594: Fix the waiting time in test_automatic_invalidation .. IMPALA-7594: Fix the waiting time in test_automatic_invalidation IMPALA-7593 increased the table invalidation timeout but the time to wait for the effect should also be increased. Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Reviewed-on: http://gerrit.cloudera.org:8080/11471 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/custom_cluster/test_automatic_invalidation.py 1 file changed, 7 insertions(+), 4 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11471 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ibb41e615e42712f9f75a4180f55270f8d4159668 Gerrit-Change-Number: 11471 Gerrit-PatchSet: 4 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3182/ -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 3 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Sep 2018 02:38:56 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-6772: Build ORC lib without libhdfspp and apply ORC-403
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11442 to look at the new patch set (#3). Change subject: IMPALA-6772: Build ORC lib without libhdfspp and apply ORC-403 .. IMPALA-6772: Build ORC lib without libhdfspp and apply ORC-403 ORC lib supports reading files in HDFS since 1.5.0. It includes libhdfs++ of Hadoop2, which causes errors when links with Impala. This patch adds a cmake option to exclude libhdfs++. Also apply ORC-403 to introduce more checks inside the ORC lib. Change-Id: I1491b558a7972c0f77b0bfeb6e9cc04fda1734e9 --- M buildall.sh M source/orc/build.sh A source/orc/orc-1.5.2-patches/0002-ORC-403-C-Add-checks-to-avoid-invalid-offsets-in-Inp.patch 3 files changed, 215 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/native-toolchain refs/changes/42/11442/3 -- To view, visit http://gerrit.cloudera.org:8080/11442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1491b558a7972c0f77b0bfeb6e9cc04fda1734e9 Gerrit-Change-Number: 11442 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong
[native-toolchain-CR] IMPALA-6772: Build ORC lib without libhdfspp and apply ORC-403
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11442 ) Change subject: IMPALA-6772: Build ORC lib without libhdfspp and apply ORC-403 .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11442/2/source/orc/orc-1.5.2-patches/0002-ORC-403-C-Add-checks-to-avoid-invalid-offsets-in-Inp.patch File source/orc/orc-1.5.2-patches/0002-ORC-403-C-Add-checks-to-avoid-invalid-offsets-in-Inp.patch: http://gerrit.cloudera.org:8080/#/c/11442/2/source/orc/orc-1.5.2-patches/0002-ORC-403-C-Add-checks-to-avoid-invalid-offsets-in-Inp.patch@20 PS2, Line 20: +++ b/c++/src/Reader.cc > I compared to the upstream patch for ORC-403 and this seems to be missing s Oh, it's a mistake. Somehow I encountered some conflicts when cherry-picking ORC-403. These lines were deleted accidently... The later fuzz-tests I ran failed due to this. Re-run the fuzz tests by while impala-py.test tests/query_test/test_scanners_fuzz.py -k orc ; do echo yes ; if [ -f core ]; then break; fi; done Will give you updates if no errors emerge in several days. -- To view, visit http://gerrit.cloudera.org:8080/11442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1491b558a7972c0f77b0bfeb6e9cc04fda1734e9 Gerrit-Change-Number: 11442 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Sep 2018 02:49:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7591: Use short name for setting owner
Adam Holley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11469 Change subject: IMPALA-7591: Use short name for setting owner .. IMPALA-7591: Use short name for setting owner This patch changes the owner to be the shortname in a Kerberized cluster. Authorization with Sentry is done using the shortname and Hive uses the shortname. Because object ownership privileges are based on owner, authorization will not work with the full name. The IOException for getShortName should only ever be thrown if the format for the name is wrong. i.e. It's actually a BadFormatString exception. Testing: - Ran all frontend, and e2e tests. Change-Id: Iab392a0cd5c0bb98442e1f7ac39f470310a21f11 --- M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java 5 files changed, 35 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11469/2 -- To view, visit http://gerrit.cloudera.org:8080/11469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iab392a0cd5c0bb98442e1f7ac39f470310a21f11 Gerrit-Change-Number: 11469 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Csaba Ringhofer
[Impala-ASF-CR] IMPALA-7591: Use short name for setting owner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11469 ) Change subject: IMPALA-7591: Use short name for setting owner .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/727/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/11469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab392a0cd5c0bb98442e1f7ac39f470310a21f11 Gerrit-Change-Number: 11469 Gerrit-PatchSet: 2 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 20 Sep 2018 03:31:54 + Gerrit-HasComments: No
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11478 to look at the new patch set (#4). Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". This instructs git to pretend that certain "minimized" and debugging Bootstrap-related files are treated as binary by git tools, especially "git grep". As a result, doing something like "git grep pause" won't yield a wall of indecipherable JavaScript. Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 --- A .gitattributes M bin/rat_exclude_files.txt 2 files changed, 21 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/78/11478/4 -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep".
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11478 ) Change subject: Mark certain vendored JS/CSS files as "binary" to avoid them showing up in "git grep". .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11478 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie8e7872e598710f871fec93f44dc0028907bfaf1 Gerrit-Change-Number: 11478 Gerrit-PatchSet: 5 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 20 Sep 2018 03:55:13 + Gerrit-HasComments: No