[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10974 ) Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/156/ : 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/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 02 Aug 2018 04:04:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6335. Allow most shell tests to run in parallel
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11045 ) Change subject: IMPALA-6335. Allow most shell tests to run in parallel .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/155/ : 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/11045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1da5739276e63a50590dfcb2b050703f8e35fec7 Gerrit-Change-Number: 11045 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 03:55:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/154/ : 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/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 12 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 02 Aug 2018 03:37:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/153/ : 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/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 11 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 02 Aug 2018 03:28:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10974 to look at the new patch set (#7). Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY This changes the permissions-checking of INSERT so that, if a partition is specified, we only verify writability of the specific explicit partition. This allows insertion into a table even if it contains one or more read-only partitions. This matches the existing behavior of LOAD DATA. New regression tests are added which failed prior to the fix. Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 --- M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M tests/metadata/test_hdfs_permissions.py M tests/query_test/test_insert_behaviour.py 7 files changed, 219 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/10974/7 -- To view, visit http://gerrit.cloudera.org:8080/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6335. Allow most shell tests to run in parallel
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11045 ) Change subject: IMPALA-6335. Allow most shell tests to run in parallel .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2902/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/11045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1da5739276e63a50590dfcb2b050703f8e35fec7 Gerrit-Change-Number: 11045 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 03:23:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6335. Allow most shell tests to run in parallel
Hello Pooja Nilangekar, Fredy Wijaya, Csaba Ringhofer, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11045 to look at the new patch set (#6). Change subject: IMPALA-6335. Allow most shell tests to run in parallel .. IMPALA-6335. Allow most shell tests to run in parallel This adds an IMPALA_HISTFILE environment variable (and --history_file argument) to the shell which overrides the default location of ~/.impalahistory for the shell history. The shell tests now override this variable to /dev/null so they don't store history. The tests that need history use a pytest fixture to use a temporary file for their history. This allows so that they can run in parallel without stomping on each other's history. This also fixes a couple flaky test which were previously missing the "execute_serially" annotation -- that annotation is no longer needed after this fix. A couple of the tests still need to be executed serially because they look at metrics such as the number of executed or running queries, and those metrics are unstable if other tests run in parallel. I tested this by running: ./bin/impala-py.test tests/shell/test_shell_interactive.py \ -m 'not execute_serially' \ -n 80 \ --random ... several times in a row on an 88-core box. Prior to the change, several would fail each time. Now they pass. Change-Id: I1da5739276e63a50590dfcb2b050703f8e35fec7 --- M shell/impala_shell.py M shell/impala_shell_config_defaults.py M shell/option_parser.py M tests/common/impala_test_suite.py M tests/custom_cluster/test_shell_interactive_reconnect.py M tests/shell/test_shell_interactive.py M tests/shell/util.py 7 files changed, 56 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/11045/6 -- To view, visit http://gerrit.cloudera.org:8080/11045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1da5739276e63a50590dfcb2b050703f8e35fec7 Gerrit-Change-Number: 11045 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 12: (3 comments) http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py File tests/failure/test_failpoints.py: http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py@152 PS11, Line 152: : > flake8: E231 missing whitespace after ':' Done http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py@157 PS11, Line 157: : > flake8: E231 missing whitespace after ':' Done http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py@168 PS11, Line 168: : > flake8: E231 missing whitespace after ':' Done -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 12 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 02 Aug 2018 02:58:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Hello Michael Ho, Joe McDonnell, Bikramjeet Vig, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10813 to look at the new patch set (#12). Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. IMPALA-7163: Implement a state machine for the QueryState class This patch adds a state machine for the QueryState class. The motivation behind this patch is to make the query lifecycle from the point of view of an executor much easier to reason about and this patch is key for a follow on patch for IMPALA-2990 where the status reporting will be per-query rather than per-fragment-instance. Currently, the state machine provides no other purpose, and it will mostly be used for IMPALA-2990. We introduce 5 possible states for the QueryState which include 3 terminal states (FINISHED, CANCELLED and ERROR) and 2 non-terminal states (PREPARING, EXECUTING). The transition from one state to the next is always handled by a single thread which is also the QueryState thread. This thread will additionally bear the purpose of sending periodic updates after IMPALA-4063, which is the primary reason behind having only this thread modify the state of the query. Counting barriers are introduced to keep a count of how many fragment instances have finished Preparing and Executing. These barriers also block until all the fragment instances have finished a respective state. The fragment instances update the query wide query status if an error is hit and unblocks the barrier if it is in the EXECUTING state. The PREPARING state blocks regardless of whether a fragment instance hit an error or not, until all the fragment instances have completed successfully or unsuccessfully, to maintain the invariant that fragment instances cannot be cancelled until the entire QueryState has finished PREPARING. The status reporting protocol has not been changed and remains exactly as it was. Testing: - Added 3 failure points in the query lifecycle using debug actions and added tests to validate the same (extension of IMPALA-7376). - Ran 'core' and 'exhaustive' tests. Future related work: 1) IMPALA-2990: Make status reporting per-query. 2) Try to logically align the FIS states with the QueryState states. 3) Consider mirroring the QueryState state machine to CoordinatorBackendState Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe --- M be/src/runtime/coordinator.cc 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 tests/failure/test_failpoints.py 6 files changed, 279 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/10813/12 -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 12 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10813 ) Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. Patch Set 11: (3 comments) http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py File tests/failure/test_failpoints.py: http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py@152 PS11, Line 152: : flake8: E231 missing whitespace after ':' http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py@157 PS11, Line 157: : flake8: E231 missing whitespace after ':' http://gerrit.cloudera.org:8080/#/c/10813/11/tests/failure/test_failpoints.py@168 PS11, Line 168: : flake8: E231 missing whitespace after ':' -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 11 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 02 Aug 2018 02:55:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Hello Michael Ho, Joe McDonnell, Bikramjeet Vig, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10813 to look at the new patch set (#11). Change subject: IMPALA-7163: Implement a state machine for the QueryState class .. IMPALA-7163: Implement a state machine for the QueryState class This patch adds a state machine for the QueryState class. The motivation behind this patch is to make the query lifecycle from the point of view of an executor much easier to reason about and this patch is key for a follow on patch for IMPALA-2990 where the status reporting will be per-query rather than per-fragment-instance. Currently, the state machine provides no other purpose, and it will mostly be used for IMPALA-2990. We introduce 5 possible states for the QueryState which include 3 terminal states (FINISHED, CANCELLED and ERROR) and 2 non-terminal states (PREPARING, EXECUTING). The transition from one state to the next is always handled by a single thread which is also the QueryState thread. This thread will additionally bear the purpose of sending periodic updates after IMPALA-4063, which is the primary reason behind having only this thread modify the state of the query. Counting barriers are introduced to keep a count of how many fragment instances have finished Preparing and Executing. These barriers also block until all the fragment instances have finished a respective state. The fragment instances update the query wide query status if an error is hit and unblocks the barrier if it is in the EXECUTING state. The PREPARING state blocks regardless of whether a fragment instance hit an error or not, until all the fragment instances have completed successfully or unsuccessfully, to maintain the invariant that fragment instances cannot be cancelled until the entire QueryState has finished PREPARING. The status reporting protocol has not been changed and remains exactly as it was. Testing: - Added 3 failure points in the query lifecycle using debug actions and added tests to validate the same (extension of IMPALA-7376). - Ran 'core' and 'exhaustive' tests. Future related work: 1) IMPALA-2990: Make status reporting per-query. 2) Try to logically align the FIS states with the QueryState states. 3) Consider mirroring the QueryState state machine to CoordinatorBackendState Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe --- M be/src/runtime/coordinator.cc 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 tests/failure/test_failpoints.py 6 files changed, 279 insertions(+), 67 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/10813/11 -- To view, visit http://gerrit.cloudera.org:8080/10813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe Gerrit-Change-Number: 10813 Gerrit-PatchSet: 11 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Sailesh Mukil has posted comments on this change. (
http://gerrit.cloudera.org:8080/10813 )
Change subject: IMPALA-7163: Implement a state machine for the QueryState class
..
Patch Set 11:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/fragment-instance-state.cc@100
PS10, Line 100: }
: // Tell the managing 'QueryState' that we're done with
executing and that we've stopped
: // the reporting thread.
: query_state_->DoneExecuting();
:
: done:
: if (!status.ok() && current_state_.Load() <=
TFInstanceExecState::WAITING_FOR_PREPARE) {
: // Tell the managing 'QueryS
> Should this be included moved to point after label done ? Otherwise, we may
Great catch. I took your suggestion and moved the ErrorDuring* calls to the
'done:' section.
I also added a debug action to fail in Open()
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@216
PS10, Line 216: ExecState o
> not used ?
Done
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@217
PS10, Line 217:
> BackendExecState old_state = backend_exec_state_; and no need for line 220
Done
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@230
PS10, Line 230: return query_status_;
: }
> nit: one line
Done
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@408
PS10, Line 408: ReleaseExecResourceRefcount();
:
ExecEnv::GetInstance()->query_exec_mgr()->ReleaseQueryState(this);
: break;
: }
: // update fragment_map_
: vector& fis_list =
fragment_map_[instance_ctx.fragment_idx];
: fis_list.push_back(fis);
: t->Detach();
: --n
> A minor suggestion is to group this loop with ErrorDuringPrepare() below so
Good point. Done.
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@435
PS10, Line 435: ReportExecStatusAux(true, thread_create_status, nullptr,
true);
> We used to wait for all fragment instances to finish preparing before repor
No, if a Cancel() arrives due to this before all the fragment instances have
finished preparing, Cancel() will block on WaitForPrepare().
We'll end up cancelling the query earlier which is good.
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@442
PS10, Line 442: BackendExecStat
> not actually used?
Done
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py
File tests/failure/test_failpoints.py:
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py@151
PS10, Line 151: self.execute_query_expect_failure(self.client, query,
: query_options={'debug_action':debug_action})
> Do we care about verifying the failure actually occurred ?
execute_query_expect_failure() does that internally.
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py@157
PS10, Line 157: query_options={'debug_action':debug_action})
:
: # Fail the fragment instance thread creation with a 0.5
probability.
: debug_action = 'FIS_FAIL
> for i in range(50):
Done
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py@165
PS10, Line 165: while i in range(50):
: try:
: self.execute_query(query,
: query_options={'de
> assert 'Query aborted:Debug Action: FIS_FAIL_THREAD_CREATION:FAIL@0.5' in s
Done
--
To view, visit http://gerrit.cloudera.org:8080/10813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
Gerrit-Change-Number: 10813
Gerrit-PatchSet: 11
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Thu, 02 Aug 2018 02:55:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11109 ) Change subject: IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/152/ : 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/11109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1b6c58a1acef9b02fcc26a04d048ee9cc47dc0ef Gerrit-Change-Number: 11109 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 02 Aug 2018 02:36:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11096 ) Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank .. IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank While Prepare()-ing a fragment instance, if we fail to initialize the runtime filter bank, we will exit FIS::Prepare() without acquiring a thread token (AcquireThreadToken()): FIS::Finalize() is called always regardless of whether the fragment instance succeeded or failed. And FIS::Finalize() tries to ReleaseThreadToken() even though it might not have gotten acquired, causing a DCHECK to be hit. This patch fixes it by making sure that no failable code is run before acquiring the thread token, thereby ensuring that the thread token is always acquired and thus avoiding the above crash. A test is added to confirm this as well. This test crashes without the code changes in this patch. Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a Reviewed-on: http://gerrit.cloudera.org:8080/11096 Reviewed-by: Sailesh Mukil Tested-by: Impala Public Jenkins --- M be/src/runtime/fragment-instance-state.cc M tests/failure/test_failpoints.py 2 files changed, 16 insertions(+), 2 deletions(-) Approvals: Sailesh Mukil: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/11096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a Gerrit-Change-Number: 11096 Gerrit-PatchSet: 4 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11096 ) Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a Gerrit-Change-Number: 11096 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Thu, 02 Aug 2018 02:20:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset
Hello Bharath Vissapragada, Vuk Ercegovac, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/11109 to review the following change. Change subject: IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset .. IMPALA-7386. Replace CatalogObjectVersionQueue with a multiset The implementation of CatalogObjectVersionQueue was based on a priority queue, which is not a good choice of data structure for the use case. This class exposes the following operations, with corresponding runtimes for the heap: - addVersion: O(lg n) - removeVersion: O(n) - getMinVersion: O(1) Given that every update of a catalog object requires removing the old version, the O(n) runtime could get quite expensive, particularly for operations like INVALIDATE METADATA which end up removing the old version of all of the objects in the catalog -- thus becoming accidentally quadratic. The new patch switches to a TreeMultiset coupled with a simple cache of the min element, with runtimes: - addVersion: O(lg n) - removeVersion: O(lg n) - getMinVersion: O(1) The downside of this patch is increased memory overhead: we are going from a PriorityQueue which has 8 bytes overhead per element to a TreeMultiset which has 48 bytes overhead per element. In a catalog with millions of tables this won't be insignificant; however, in such a catalog the performance savings are probably critical. According to a quick JMH benchmark, removal of an element from a PriorityQueue with 1M elements takes about 3.5ms, so an operation which invalidates all million tables would take about 3500 seconds without this patch. Meanwhile, the same benchmark using a TreeSet takes about 28ns per removal, so the whole invalidation would take about 28ms. This patch also makes the data structure synchronized, just for simplicity's sake, since uncontended locks are very cheap in Java. Change-Id: I1b6c58a1acef9b02fcc26a04d048ee9cc47dc0ef --- M fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/catalog/CatalogObjectCache.java D fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionQueue.java A fe/src/main/java/org/apache/impala/catalog/CatalogObjectVersionSet.java M fe/src/main/java/org/apache/impala/catalog/ImpaladCatalog.java A fe/src/test/java/org/apache/impala/catalog/CatalogObjectVersionSetTest.java 6 files changed, 214 insertions(+), 92 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/11109/1 -- To view, visit http://gerrit.cloudera.org:8080/11109 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1b6c58a1acef9b02fcc26a04d048ee9cc47dc0ef Gerrit-Change-Number: 11109 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7297: soft memory limit for reservation increases
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10988 ) Change subject: IMPALA-7297: soft memory limit for reservation increases .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39dcf2dd870a59c8b8cda4488fe41ce936d715ac Gerrit-Change-Number: 10988 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 01:46:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7203. Support UDFs in LocalCatalog
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/11053 ) Change subject: IMPALA-7203. Support UDFs in LocalCatalog .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6130d07b9c641525382a618a9f8da048c7ae75ed Gerrit-Change-Number: 11053 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 02 Aug 2018 01:32:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10974 ) Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 02 Aug 2018 01:32:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7347: Update tests to accomodate HIVE-18118
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11108 ) Change subject: IMPALA-7347: Update tests to accomodate HIVE-18118 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6aae402dd38374de90b35c32166a9507e6eb29f9 Gerrit-Change-Number: 11108 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 02 Aug 2018 01:21:54 + Gerrit-HasComments: No
[Impala-ASF-CR] Bump Kudu version to 5211897
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11071 ) Change subject: Bump Kudu version to 5211897 .. Bump Kudu version to 5211897 This requires changing one error message in a test due to a change in the error message on the Kudu side. Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Reviewed-on: http://gerrit.cloudera.org:8080/11071 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M bin/impala-config.sh M tests/query_test/test_kudu.py 2 files changed, 3 insertions(+), 3 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Gerrit-Change-Number: 11071 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Bump Kudu version to 5211897
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11071 ) Change subject: Bump Kudu version to 5211897 .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Gerrit-Change-Number: 11071 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 02 Aug 2018 01:18:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA HOME
Tianyi Wang has abandoned this change. ( http://gerrit.cloudera.org:8080/11105 ) Change subject: IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA_HOME .. Abandoned Oops, it works on my machine but not on jenkins. -- To view, visit http://gerrit.cloudera.org:8080/11105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I79501fbc762176674bb1c0dde4196a592aee49b2 Gerrit-Change-Number: 11105 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7381: Prevent build failure after switching to new CDH BUILD NUMBER
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11099 ) Change subject: IMPALA-7381: Prevent build failure after switching to new CDH_BUILD_NUMBER .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/150/ : 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/11099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0ad9c2258663d3bd7470e6df921041d1ca0c0be Gerrit-Change-Number: 11099 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 02 Aug 2018 00:59:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7347: Update tests to accomodate HIVE-18118
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11108 Change subject: IMPALA-7347: Update tests to accomodate HIVE-18118 .. IMPALA-7347: Update tests to accomodate HIVE-18118 HIVE-18118 adds 'numFilesErasureCoded' to table properties. This patch addes it to test_show_create_table to work with the latest Hive. Change-Id: I6aae402dd38374de90b35c32166a9507e6eb29f9 --- M bin/impala-config.sh M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test 2 files changed, 3 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/11108/1 -- To view, visit http://gerrit.cloudera.org:8080/11108 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6aae402dd38374de90b35c32166a9507e6eb29f9 Gerrit-Change-Number: 11108 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang
[Impala-ASF-CR] IMPALA-7381: Prevent build failure after switching to new CDH BUILD NUMBER
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11099 ) Change subject: IMPALA-7381: Prevent build failure after switching to new CDH_BUILD_NUMBER .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11099/1/bin/clean.sh File bin/clean.sh: http://gerrit.cloudera.org:8080/#/c/11099/1/bin/clean.sh@81 PS1, Line 81: > Thinking about this, one use case is to use a single Impala checkout and sw I updated the patch to append CDH_BUILD_NUMBER into cdh_components so we no longer need to delete it. I also implemented a simple way of detecting if CDH_BUILD_NUMBER changes and only perform mvn -U if it changes. Please review the patch and let me know what you think. -- To view, visit http://gerrit.cloudera.org:8080/11099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0ad9c2258663d3bd7470e6df921041d1ca0c0be Gerrit-Change-Number: 11099 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 02 Aug 2018 00:16:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7381: Prevent build failure after switching to new CDH BUILD NUMBER
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11099 ) Change subject: IMPALA-7381: Prevent build failure after switching to new CDH_BUILD_NUMBER .. IMPALA-7381: Prevent build failure after switching to new CDH_BUILD_NUMBER Switching to a new CDH_BUILD_NUMBER requires downloading new CDH components as well as forcing Maven to update its local repository. This patch updates the CDH_COMPONENTS_HOME to include the CDH_BUILD_NUMBER which will automatically download the new CDH components after switching to a new CDH_BUILD_NUMBER. When running a build if it detects that a new CDH_BUILD_NUMBER has changed, the build will force an update to the local Maven repository. This helps to prevent build failure even on a fresh Git clone due to stale local Maven repository. Testing: - Manually tested by running buildall.sh with different CDH_BUILD_NUMBER Change-Id: Ib0ad9c2258663d3bd7470e6df921041d1ca0c0be --- M .gitignore M bin/impala-config.sh M buildall.sh 3 files changed, 14 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/11099/2 -- To view, visit http://gerrit.cloudera.org:8080/11099 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib0ad9c2258663d3bd7470e6df921041d1ca0c0be Gerrit-Change-Number: 11099 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/10813 )
Change subject: IMPALA-7163: Implement a state machine for the QueryState class
..
Patch Set 10:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/fragment-instance-state.cc@100
PS10, Line 100: if (!status.ok()) {
: // Tell the managing 'QueryState' that we hit an error
during execution.
: query_state_->ErrorDuringExecute(status, instance_id());
: goto done;
: }
: // Tell the managing 'QueryState' that we're done with
executing and that we've stopped
: // the reporting thread.
: query_state_->DoneExecuting();
Should this be included moved to point after label done ? Otherwise, we may
hang in case Open() fails ?
May be this warrants a debug action which triggers failure() in Open().
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@216
PS10, Line 216: ret_status;
not used ?
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@217
PS10, Line 217: BackendExecState old_state;
BackendExecState old_state = backend_exec_state_; and no need for line 220
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@230
PS10, Line 230: BackendExecState::EXECUTING :
: BackendExecState::FINISHED;
nit: one line
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@408
PS10, Line 408: // We failed to start 'num_unstarted_instances', so make
sure to notify
: // 'instances_prepared_barrier_' 'num_unstarted_instances
- 1' times, to unblock
: // 'WaitForPrepare(). The last remaining notification
will be set by the error
: // handling logic once we've exited this loop so that the
we may have the query
: // wide status reflect the 'thread_create_status'.
: while (num_unstarted_instances > 1) {
: DonePreparing();
: --num_unstarted_instances;
: }
A minor suggestion is to group this loop with ErrorDuringPrepare() below so
it's easier to see that we have updated num_unstarted_instances times.
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@435
PS10, Line 435: ErrorDuringPrepare(thread_create_status, TUniqueId());
We used to wait for all fragment instances to finish preparing before reporting
errors. This patch stops doing that. Does it matter ?
http://gerrit.cloudera.org:8080/#/c/10813/10/be/src/runtime/query-state.cc@442
PS10, Line 442: all_fis_status;
not actually used?
WaitForPrepare() below can become discard_result(WaitForPrepare());
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py
File tests/failure/test_failpoints.py:
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py@151
PS10, Line 151: self.execute_query_expect_failure(self.client, query,
: query_options={'debug_action':debug_action})
Do we care about verifying the failure actually occurred ?
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py@157
PS10, Line 157: x = 0
: # Since there's only a 50% chance of fragment instance
thread creation failure,
: # we attempt to catch a query failure up to a very
conservative maximum of 50 tries.
: while x in range(0, 50):
for i in range(50):
http://gerrit.cloudera.org:8080/#/c/10813/10/tests/failure/test_failpoints.py@165
PS10, Line 165: if 'Query aborted:Debug Action:
FIS_FAIL_THREAD_CREATION:FAIL@0.5' in str(e):
: break
: else:
: assert False, str(e)
assert 'Query aborted:Debug Action: FIS_FAIL_THREAD_CREATION:FAIL@0.5' in
str(e), str(e)
--
To view, visit http://gerrit.cloudera.org:8080/10813
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec5670a7db83ecae4656d7bb2ea372d3767ba7fe
Gerrit-Change-Number: 10813
Gerrit-PatchSet: 10
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Thu, 02 Aug 2018 00:06:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/149/ : 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/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 02 Aug 2018 00:03:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA HOME
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11105 ) Change subject: IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA_HOME .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/147/ : 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/11105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I79501fbc762176674bb1c0dde4196a592aee49b2 Gerrit-Change-Number: 11105 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Thu, 02 Aug 2018 00:03:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7385: Fix test-with-docker errors having to do with time zones.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11106 ) Change subject: IMPALA-7385: Fix test-with-docker errors having to do with time zones. .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/148/ : 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/11106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia9facfd9741806e7dbb868d8d06d9296bf86e52f Gerrit-Change-Number: 11106 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Laszlo Gaal Gerrit-Comment-Date: Wed, 01 Aug 2018 23:59:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7383: Configurable METASTORE DB defaulting to escaped IMPALA HOME
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11104 ) Change subject: IMPALA-7383: Configurable METASTORE_DB defaulting to escaped IMPALA_HOME .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/146/ : 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/11104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 Gerrit-Change-Number: 11104 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 01 Aug 2018 23:49:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7308. Support Avro tables in LocalCatalog
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10970 ) Change subject: IMPALA-7308. Support Avro tables in LocalCatalog .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10970/6/tests/metadata/test_partition_metadata.py File tests/metadata/test_partition_metadata.py: http://gerrit.cloudera.org:8080/#/c/10970/6/tests/metadata/test_partition_metadata.py@143 PS6, Line 143: s > flake8: E125 continuation line with same indent as next logical line This check should be disabled from now onwards. http://gerrit.cloudera.org:8080/#/c/10970/6/tests/metadata/test_partition_metadata.py@145 PS6, Line 145: > flake8: E125 continuation line with same indent as next logical line This check should be disabled from now onwards. -- To view, visit http://gerrit.cloudera.org:8080/10970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4b86c8203271b773a711ed77558ec3e3070cb69 Gerrit-Change-Number: 10970 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 23:46:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Tim Armstrong has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/11102 )
Change subject: IMPALA-7317: loosen flake8 rules
..
IMPALA-7317: loosen flake8 rules
I got feedback that the flake8 rules are a little too strict, so based
on the d...@impala.apache.org discussion am disabling some of them.
I left E302 "expected 2 blank lines, found 1" enabled since I think it
would force our formatting to be more consistent, but I'm open to
changing that.
I used the following command to get a sense of the number of existing
violations:
EXCLUDES=ext-py,shell/build,gerrit_critic_venv,infra/python/env
EXCLUDES+=,shell/pkg_resources.py,shell/thrift_sasl.py,gen-py
EXCLUDES+=,TPC-DS-master,toolchain
./bin/impala-flake8 --exclude=${EXCLUDES}
Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Reviewed-on: http://gerrit.cloudera.org:8080/11102
Reviewed-by: Tim Armstrong
Tested-by: Tim Armstrong
---
M bin/jenkins/critique-gerrit-review.py
M setup.cfg
2 files changed, 7 insertions(+), 3 deletions(-)
Approvals:
Tim Armstrong: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 3: Verified+1 Code-Review+2 Carry +2 Ran ./bin/jenkins/critique-gerrit-review.py --dryrun to test (since it's not exercised precommit). -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 23:44:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/11102/2/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/2/setup.cfg@23 PS2, Line 23: ignore = E111,E114,E121,E125,E128,E701 > E127 is mentioned L19 but absent L23. Done -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 23:43:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7203. Support UDFs in LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11053 ) Change subject: IMPALA-7203. Support UDFs in LocalCatalog .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/144/ : 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/11053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6130d07b9c641525382a618a9f8da048c7ae75ed Gerrit-Change-Number: 11053 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 23:43:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Hello Michael Brown, David Knupp, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11102
to look at the new patch set (#3).
Change subject: IMPALA-7317: loosen flake8 rules
..
IMPALA-7317: loosen flake8 rules
I got feedback that the flake8 rules are a little too strict, so based
on the d...@impala.apache.org discussion am disabling some of them.
I left E302 "expected 2 blank lines, found 1" enabled since I think it
would force our formatting to be more consistent, but I'm open to
changing that.
I used the following command to get a sense of the number of existing
violations:
EXCLUDES=ext-py,shell/build,gerrit_critic_venv,infra/python/env
EXCLUDES+=,shell/pkg_resources.py,shell/thrift_sasl.py,gen-py
EXCLUDES+=,TPC-DS-master,toolchain
./bin/impala-flake8 --exclude=${EXCLUDES}
Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
---
M bin/jenkins/critique-gerrit-review.py
M setup.cfg
2 files changed, 7 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/11102/3
--
To view, visit http://gerrit.cloudera.org:8080/11102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/145/ : 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/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 23:42:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7308. Support Avro tables in LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10970 ) Change subject: IMPALA-7308. Support Avro tables in LocalCatalog .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/140/ : 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/10970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4b86c8203271b773a711ed77558ec3e3070cb69 Gerrit-Change-Number: 10970 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 23:36:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7385: Fix test-with-docker errors having to do with time zones.
Philip Zeyliger has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11106 Change subject: IMPALA-7385: Fix test-with-docker errors having to do with time zones. .. IMPALA-7385: Fix test-with-docker errors having to do with time zones. ExprTest.TimestampFunctions, query_test.test_scanners.TestOrc.test_type_conversions, and query_test.test_queries.TestHdfsQueries.test_hdfs_scan_node were all failing when using test-with-docker with mismatched dates. As it turns out, there is code that calls readlink(/etc/timezone) and parses the output to identify the current timezone name. This is described in localtime(5) on Ubuntu16: It should be an absolute or relative symbolic link pointing to /usr/share/zoneinfo/, followed by a timezone identifier such as "Europe/Berlin" or "Etc/UTC". ... Because the timezone identifier is extracted from the symlink target name of /etc/localtime, this file may not be a normal file or hardlink." To honor this requirement, and to make the tests pass, I re-jiggered how I pass the time zone information from the host into the container. The previously failing tests now pass. Change-Id: Ia9facfd9741806e7dbb868d8d06d9296bf86e52f --- M docker/entrypoint.sh M docker/test-with-docker.py 2 files changed, 16 insertions(+), 21 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/11106/1 -- To view, visit http://gerrit.cloudera.org:8080/11106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia9facfd9741806e7dbb868d8d06d9296bf86e52f Gerrit-Change-Number: 11106 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10974 ) Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/142/ : 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/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 23:30:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 2: Code-Review+2 (1 comment) Feel free to carry +2 after addressing the comment. http://gerrit.cloudera.org:8080/#/c/11102/2/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/2/setup.cfg@23 PS2, Line 23: ignore = E111,E114,E121,E125,E128,E701 E127 is mentioned L19 but absent L23. -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 23:29:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6994: Avoid reloading a table's HMS data for file-only operations
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10587 ) Change subject: IMPALA-6994: Avoid reloading a table's HMS data for file-only operations .. Patch Set 4: (10 comments) http://gerrit.cloudera.org:8080/#/c/10587/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10587/4//COMMIT_MSG@9 PS4, Line 9: new row this should apply more generally to new files, not just rows? either way, the wording here is inconsistent with the title, which refers to "file-only operations". http://gerrit.cloudera.org:8080/#/c/10587/4//COMMIT_MSG@17 PS4, Line 17: the all? which partitions? http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1417 PS4, Line 1417: a nit: an http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1420 PS4, Line 1420: by calling nit: from http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@254 PS4, Line 254: reloads the table schema from Hive Meta Store can in-line this comment directly on L259. same with the other descriptions of what each flag does when set. doing so will remove some redundancy in the comment. http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@585 PS4, Line 585: noneOf(MetaDataLoadFlag.class); : flags.add simplify to .of(...) since we're always adding RELOAD_PARTITION_METADATA. http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@663 PS4, Line 663: MetaDataLoadFlag of type 'RELOAD_TABLE_METADATA', 'RELOAD_PARTITION_METADATA', :* 'RELOAD_FILE_METADATA' replace with just flags. http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@670 PS4, Line 670: msTbl why is this pulled out now as a parameter. I see one place where its used-- what requires this change? if its really needed, please add a comment (e.g., what does a null msTbl do?) http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3484 PS4, Line 3484: noneOf(MetaDataLoadFlag.class); : flags.add(MetaDataLoadFlag.RELOAD_FILE_METADATA) simplify to of(...) since the flag on L3485 is always included. http://gerrit.cloudera.org:8080/#/c/10587/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3486 PS4, Line 3486: == true remove -- To view, visit http://gerrit.cloudera.org:8080/10587 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I331bb0371fde287f43a85b025b4f98cb45f3eb3c Gerrit-Change-Number: 10587 Gerrit-PatchSet: 4 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 23:26:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10971 ) Change subject: IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/141/ : 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/10971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I479b7f517091dd558768601e1e0704a1902b78a5 Gerrit-Change-Number: 10971 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 23:24:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA HOME
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11105 ) Change subject: IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA_HOME .. Patch Set 1: Code-Review+2 This looks fine to me if it works. -- To view, visit http://gerrit.cloudera.org:8080/11105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I79501fbc762176674bb1c0dde4196a592aee49b2 Gerrit-Change-Number: 11105 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 01 Aug 2018 23:23:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA HOME
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11105 Change subject: IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA_HOME .. IMPALA-7384: Move /var/lib/hadoop-hdfs into IMPALA_HOME Currently the mini-cluster uses /var/lib/hadoop-hdfs as HDFS shortcut socket dir. It's a hard-coded path, requires root privilege to create, is a leaky resource, and is one of the many reasons preventing developers to run multiple mini-clusters concurrently. We should put it into the testdata/cluster directory instead. Change-Id: I79501fbc762176674bb1c0dde4196a592aee49b2 --- M bin/bootstrap_system.sh M testdata/cluster/admin M testdata/cluster/node_templates/common/etc/hadoop/conf/hdfs-site.xml.tmpl 3 files changed, 3 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/11105/1 -- To view, visit http://gerrit.cloudera.org:8080/11105 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I79501fbc762176674bb1c0dde4196a592aee49b2 Gerrit-Change-Number: 11105 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang
[Impala-ASF-CR] IMPALA-7383: Configurable METASTORE DB defaulting to escaped IMPALA HOME
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11104 Change subject: IMPALA-7383: Configurable METASTORE_DB defaulting to escaped IMPALA_HOME .. IMPALA-7383: Configurable METASTORE_DB defaulting to escaped IMPALA_HOME Some developers keep multiple impala repos on their disk. Isolating METASTORE_DB may help with switching between those repos without reloading the data. This patch makes METASTORE_DB configurable and default to an escaped IMPALA_HOME path. Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 --- M bin/impala-config.sh 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/11104/1 -- To view, visit http://gerrit.cloudera.org:8080/11104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I190d657cb95dfdf73ebd05e5dd24ef2a8e3156b8 Gerrit-Change-Number: 11104 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang
[Impala-ASF-CR] IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11027 ) Change subject: IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/143/ : 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/11027 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83e5ebc214d6620d165e13f8cc80f8fdda100734 Gerrit-Change-Number: 11027 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 23:11:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Hello Michael Brown, David Knupp, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11102
to look at the new patch set (#2).
Change subject: IMPALA-7317: loosen flake8 rules
..
IMPALA-7317: loosen flake8 rules
I got feedback that the flake8 rules are a little too strict, so based
on the d...@impala.apache.org discussion am disabling some of them.
I left E302 "expected 2 blank lines, found 1" enabled since I think it
would force our formatting to be more consistent, but I'm open to
changing that.
I used the following command to get a sense of the number of existing
violations:
EXCLUDES=ext-py,shell/build,gerrit_critic_venv,infra/python/env
EXCLUDES+=,shell/pkg_resources.py,shell/thrift_sasl.py,gen-py
EXCLUDES+=,TPC-DS-master,toolchain
./bin/impala-flake8 --exclude=${EXCLUDES}
Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
---
M bin/jenkins/critique-gerrit-review.py
M setup.cfg
2 files changed, 7 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/11102/2
--
To view, visit http://gerrit.cloudera.org:8080/11102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Brown
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@23 PS1, Line 23: ignore = E111,E114,E121,E128,E701 > OK, any bot is better than no bot, so let's go with these exclusions includ Done -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 23:08:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11096 ) Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2900/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11096 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a Gerrit-Change-Number: 11096 Gerrit-PatchSet: 3 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Wed, 01 Aug 2018 23:01:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank
Sailesh Mukil has posted comments on this change. (
http://gerrit.cloudera.org:8080/11096 )
Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to
initialize the filter bank
..
Patch Set 3: Code-Review+2
(1 comment)
Carry +2.
http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py
File tests/failure/test_failpoints.py:
http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py@147
PS2, Line 147: query_options={'debug_action':debug_action})
> Should we also verify that the query actually failed ?
It is already doing that since the function called is:
execute_query_expect_failure()
--
To view, visit http://gerrit.cloudera.org:8080/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Wed, 01 Aug 2018 23:01:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7140 (part 9): add support for SHOW FILES in LocalFsTable
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10973 ) Change subject: IMPALA-7140 (part 9): add support for SHOW FILES in LocalFsTable .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10973 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I143bea9f72b6a25ae2545663e06f4849b58533ba Gerrit-Change-Number: 10973 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 22:47:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7376: DCHECK hit if a fragment instance fails to initialize the filter bank
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/11096 )
Change subject: IMPALA-7376: DCHECK hit if a fragment instance fails to
initialize the filter bank
..
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py
File tests/failure/test_failpoints.py:
http://gerrit.cloudera.org:8080/#/c/11096/2/tests/failure/test_failpoints.py@147
PS2, Line 147: query_options={'debug_action':debug_action})
Should we also verify that the query actually failed ?
--
To view, visit http://gerrit.cloudera.org:8080/11096
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1d6e7afc18fe2f0e1e29d2bd8a5f804a78f7043a
Gerrit-Change-Number: 11096
Gerrit-PatchSet: 2
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho
Gerrit-Comment-Date: Wed, 01 Aug 2018 22:46:09 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10974 ) Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10974/6/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10974/6/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@435 PS6, Line 435: * If 'partitionKeyValues' is null, the user should have write access to all partitions line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/10974/6/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@470 PS6, Line 470: // No explicit partition was specified. Need to ensure that write access is available line too long (93 > 90) -- To view, visit http://gerrit.cloudera.org:8080/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 22:34:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7308. Support Avro tables in LocalCatalog
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10970 ) Change subject: IMPALA-7308. Support Avro tables in LocalCatalog .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/10970/6/tests/metadata/test_partition_metadata.py File tests/metadata/test_partition_metadata.py: http://gerrit.cloudera.org:8080/#/c/10970/6/tests/metadata/test_partition_metadata.py@143 PS6, Line 143: s flake8: E125 continuation line with same indent as next logical line http://gerrit.cloudera.org:8080/#/c/10970/6/tests/metadata/test_partition_metadata.py@145 PS6, Line 145: flake8: E125 continuation line with same indent as next logical line -- To view, visit http://gerrit.cloudera.org:8080/10970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie4b86c8203271b773a711ed77558ec3e3070cb69 Gerrit-Change-Number: 10970 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 22:33:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7203. Support UDFs in LocalCatalog
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11053 to look at the new patch set (#2). Change subject: IMPALA-7203. Support UDFs in LocalCatalog .. IMPALA-7203. Support UDFs in LocalCatalog This adds support to LocalCatalog to load persistent UDFs (both Java and native) from the HMS. Transient UDFs are not supported, since, without a central component to store them between restarts, there isn't any clear path to doing so. The various UDF tests do not yet pass reliably with this change since so many of them rely on the transient Java UDF functionality. This is a legacy feature that isn't commonly used today, and we may not be able to support it in LocalCatalog. For now, I tested manually that I could create, drop, and run some UDFs and that they show up in 'show functions'. Change-Id: I6130d07b9c641525382a618a9f8da048c7ae75ed --- M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/Db.java M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java M fe/src/main/java/org/apache/impala/catalog/local/MetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java A fe/src/main/java/org/apache/impala/util/FunctionUtils.java M fe/src/main/java/org/apache/impala/util/PatternMatcher.java 9 files changed, 541 insertions(+), 255 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/53/11053/2 -- To view, visit http://gerrit.cloudera.org:8080/11053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6130d07b9c641525382a618a9f8da048c7ae75ed Gerrit-Change-Number: 11053 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog
Hello Bharath Vissapragada, Tianyi Wang, Vuk Ercegovac, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10971 to look at the new patch set (#6). Change subject: IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog .. IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog Tested by running 'run-tests.py -k stats_extrap' and the tests passed. Change-Id: I479b7f517091dd558768601e1e0704a1902b78a5 --- M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/test/java/org/apache/impala/planner/StatsExtrapolationTest.java 6 files changed, 62 insertions(+), 70 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/10971/6 -- To view, visit http://gerrit.cloudera.org:8080/10971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I479b7f517091dd558768601e1e0704a1902b78a5 Gerrit-Change-Number: 10971 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Hello Bharath Vissapragada, Tianyi Wang, Impala Public Jenkins, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10974 to look at the new patch set (#6). Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY This changes the permissions-checking of INSERT so that, if a partition is specified, we only verify writability of the specific explicit partition. This allows insertion into a table even if it contains one or more read-only partitions. This matches the existing behavior of LOAD DATA. New regression tests are added which failed prior to the fix. Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 --- M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M tests/metadata/test_hdfs_permissions.py M tests/query_test/test_insert_behaviour.py 7 files changed, 219 insertions(+), 77 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/10974/6 -- To view, visit http://gerrit.cloudera.org:8080/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7308. Support Avro tables in LocalCatalog
Hello Bharath Vissapragada, Tianyi Wang, Vuk Ercegovac, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10970 to look at the new patch set (#6). Change subject: IMPALA-7308. Support Avro tables in LocalCatalog .. IMPALA-7308. Support Avro tables in LocalCatalog This adds support for loading Avro-formatted tables in LocalCatalog. In the case that the table properties indicate a table is Avro-formatted, the semantics are identical to the existing catalog implementation: - if an explicit avro schema is specified, it overrides the schema provided by the HMS - if no explicit avro schema is specified, one is inferred, and then the inferred schema takes the place of the one provided by the HMS (thus promoting columns like TINYINT to INT) - on COMPUTE STATS, if any discrepancy is discovered between the HMS schema and the inferred schema, an error is emitted. The semantics for LocalCatalog are slightly different in the case of tables which have not been configured as Avro format on the table level: The existing implementation has the behavior that, when a table is loaded, all partitions are inspected, and, if any partition is discovered with Avro format, the above rules are applied. This has some very unexpected results, described in an earlier email to d...@impala.apache.org [1]. To summarize that email thread, the existing behavior was decided to be unintuitive and inconsistent with Hive. Additionally, this behavior requires loading all partitions up-front, which gets in the goal of lazy/granular metadata loading in LocalCatalog. Thus, the LocalCatalog implementation differs as follows: - the "schema override" behavior ONLY occurs if the Avro file format has been selected at a table level. - if an Avro partition is added to a non-Avro table, and that partition has a schema that isn't compatible with the table's schema, an error will occur on read. The thread additionally discusses adding an error message on "alter" to prevent users from adding an Avro partition to a table with an incompatible schema. To keep the scope of this patch minimal, that is not yet implemented here. I filed IMPALA-7309 to change the behavior of the existing catalog implementation to match. A new test verifies the behavior, set to 'xfail' when running on the existing catalog implementation. [1] https://lists.apache.org/thread.html/fb68c54bd66a40982ee17f9f16f87a4112220a5df035a311bda310f1@%3Cdev.impala.apache.org%3E Change-Id: Ie4b86c8203271b773a711ed77558ec3e3070cb69 --- M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java M fe/src/main/java/org/apache/impala/catalog/FeFsTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/main/java/org/apache/impala/util/AvroSchemaUtils.java M fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java A testdata/workloads/functional-query/queries/QueryTest/incompatible_avro_partition.test M tests/common/custom_cluster_test_suite.py M tests/conftest.py M tests/metadata/test_partition_metadata.py M tests/query_test/test_avro_schema_resolution.py 12 files changed, 270 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/70/10970/6 -- To view, visit http://gerrit.cloudera.org:8080/10970 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie4b86c8203271b773a711ed77558ec3e3070cb69 Gerrit-Change-Number: 10970 Gerrit-PatchSet: 6 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7297: soft memory limit for reservation increases
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10988 ) Change subject: IMPALA-7297: soft memory limit for reservation increases .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2899/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39dcf2dd870a59c8b8cda4488fe41ce936d715ac Gerrit-Change-Number: 10988 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 22:30:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7203. Support UDFs in LocalCatalog
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11053 ) Change subject: IMPALA-7203. Support UDFs in LocalCatalog .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11053/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11053/1//COMMIT_MSG@1 PS1, Line 1: Parent: 0af4661c (IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded) > In my understanding the transient UDF is a legacy feature and is not widely that's correct http://gerrit.cloudera.org:8080/#/c/11053/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11053/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@166 PS1, Line 166: MetaException > No need to declare MetaException because it extends TException. There are m fixed the new ones in this patch -- To view, visit http://gerrit.cloudera.org:8080/11053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6130d07b9c641525382a618a9f8da048c7ae75ed Gerrit-Change-Number: 11053 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 22:24:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7381: Prevent build failure after switching to new CDH BUILD NUMBER
Joe McDonnell has posted comments on this change. (
http://gerrit.cloudera.org:8080/11099 )
Change subject: IMPALA-7381: Prevent build failure after switching to new
CDH_BUILD_NUMBER
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11099/1/bin/clean.sh
File bin/clean.sh:
http://gerrit.cloudera.org:8080/#/c/11099/1/bin/clean.sh@81
PS1, Line 81: rm -rf ${IMPALA_HOME}/toolchain
> Removing toolchain/cdh_components is necessary to make sure the CDH compone
Thinking about this, one use case is to use a single Impala checkout and switch
between branches. A developer can be working on the latest master branch with
CDH_BUILD_NUMBER=X and then switch branches and work on a branch that hasn't
been rebased with CDH_BUILD_NUBMER=X-1. Sometimes when switching branches, a
clean build is the most reliable way to make sure things don't interfere with
each other. A clean build is not always necessary, but when you need a clean
build, there is usually no working around it.
Downloading the entire toolchain is about 3165MB of network. Downloading CDH
alone is about 1580MB of network. (This was a surprise to me.)
If someone already has a working system and doesn't rebase, it would be nice to
avoid extra network activity.
I agree we need to do something so people don't need to manually handle
CDH_BUILD_NUMBER changes. Is it difficult to only do these things only if the
number changes?
I think incorporating the CDH_BUILD_NUMBER into cdh_components would go a long
way to helping, as that can avoid deleting the toolchain. It might be more
complicated to detect when to do mvn -U.
--
To view, visit http://gerrit.cloudera.org:8080/11099
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0ad9c2258663d3bd7470e6df921041d1ca0c0be
Gerrit-Change-Number: 11099
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 01 Aug 2018 22:21:52 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@23 PS1, Line 23: ignore = E111,E114,E121,E128,E701 > I'm not sure the "wrong" example is really wrong, at least according to fla It sounds as if we agree that by choosing 2-space indents, we've confused the tool. However, it is possible to make the tool happy without exclusions. That is what I was trying to point out. -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 22:20:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11027 ) Change subject: IMPALA-7320. Avoid calling getFileStatus() for each partition when table is loaded .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/11027/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java: http://gerrit.cloudera.org:8080/#/c/11027/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1566 PS3, Line 1566: void > nit: Java-style comment: Returning FsPermissionCache (vs) passing the outp Done http://gerrit.cloudera.org:8080/#/c/11027/3/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java File fe/src/main/java/org/apache/impala/util/FsPermissionCache.java: http://gerrit.cloudera.org:8080/#/c/11027/3/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java@45 PS3, Line 45: FsPermissionChecker checker = FsPermissionChecker.getInstance(); > Make it a static class member may be? this is already just an accessor for a static instance in FsPermissionChecker, so there isn't really any benefit to "caching" the result -- To view, visit http://gerrit.cloudera.org:8080/11027 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I83e5ebc214d6620d165e13f8cc80f8fdda100734 Gerrit-Change-Number: 11027 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 22:20:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7307 (part 2). Support TABLESAMPLE in LocalCatalog
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10972 ) Change subject: IMPALA-7307 (part 2). Support TABLESAMPLE in LocalCatalog .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2f7baf05f16c6389ed900e0459708005ab44491e Gerrit-Change-Number: 10972 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 22:17:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ ONLY
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10974 ) Change subject: IMPALA-7311. Allow INSERT on writable partitions even if some other partition is READ_ONLY .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10974/5/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10974/5/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@470 PS5, Line 470: / No explicit partition was specified. Need to ensure that write access is available : // to all partitions. > good catch -- guess we need to be able to write to all existing partitions Fixed and added tests -- To view, visit http://gerrit.cloudera.org:8080/10974 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dd81100ae73fcabdbfaf679c20cea7dc102cd13 Gerrit-Change-Number: 10974 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 22:15:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@23 PS1, Line 23: ignore = E111,E114,E121,E128,E701 > The correct formatting for the above is this: I'm not sure the "wrong" example is really wrong, at least according to flake8. With 4-space whitespace, it is fine with this: def test_incompatible_avro_partition_in_non_avro_table( self, vector, unique_database, main_table_format): pass (in fact, autopep8 corrects it to that). However, if I replace the 4-indentation with 2-indentation above, I get: /tmp/test.py:2:5: E125 continuation line with same indent as next logical line self, vector, unique_database, main_table_format): ^ /tmp/test.py:3:3: E111 indentation is not a multiple of four pass ^ So, in our case, we've silenced E111, but then the confusion over indentation makes it falsely trigger E125, no? -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 22:14:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Bump Kudu version to 5211897
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11071 ) Change subject: Bump Kudu version to 5211897 .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2898/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/11071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Gerrit-Change-Number: 11071 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 22:05:38 + Gerrit-HasComments: No
[Impala-ASF-CR] Bump Kudu version to 5211897
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11071 ) Change subject: Bump Kudu version to 5211897 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/11071 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic118b8ddbea8cbf0412516a0450e315a7a3c62e3 Gerrit-Change-Number: 11071 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 22:04:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@19 PS1, Line 19: # E121,E127,E128: ignored to allow some of the more common idioms for multi-line statement : # indentation used in the Impala code. Excluding these leads to a broader interpretation of how Python whitespace should be applied and thus inconsistencies in style. I don't think keeping them enforced would produce an undue burden on submitters, because we are using --diff and not "punishing" a submitter for existing code that violates these. However, I won't block this if the community prefers. http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@21 PS1, Line 21: # E701: ignored to allow if statement body on same line as conditional I can live with this, as PEP-008 doesn't forbid it outright. -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 21:58:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Michael Brown has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@23 PS1, Line 23: ignore = E111,E114,E121,E128,E701 > I'm getting issues with E125 also with code like this: The correct formatting for the above is this: def long_function_name_so_i_need_to_wrap_params( long, parameter, list ): pass Also acceptable: def long_function_name_so_i_need_to_wrap_params2(long, parameter, list): pass def long_function_name_so_i_need_to_wrap_params3(long, parameter, list): pass Wrong, and I see this a lot: def long_function_name_so_i_need_to_wrap_params4(long, parameter, list): pass Note that with the very first example, a call is treated the way you'd prefer. This passes: long_function_name_so_i_need_to_wrap_params( 1, 1, 1) -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 21:55:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10971 ) Change subject: IMPALA-7307 (part 1). Support stats extrapolation in LocalCatalog .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10971/5/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10971/5/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@197 PS5, Line 197: public static > Modifier 'public' is redundant for interface fields Done -- To view, visit http://gerrit.cloudera.org:8080/10971 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I479b7f517091dd558768601e1e0704a1902b78a5 Gerrit-Change-Number: 10971 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 01 Aug 2018 21:38:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg File setup.cfg: http://gerrit.cloudera.org:8080/#/c/11102/1/setup.cfg@23 PS1, Line 23: ignore = E111,E114,E121,E128,E701 I'm getting issues with E125 also with code like this: def long_function_name_so_i_need_to_wrap_params( long, parameter, list): pass It raises "E125 continuation line with same indent as next logical line" on the wrapped parameter list, because it sees it as +4 indentation (one indentation level) from the definition, and expects that the body of the function will also be +4. But, since we use +2 indent level in general, the complaint actually doesn't make sense. Perhaps we should disable E125 or figure out some way to hack flake8 to understand the +2 base indentation? -- To view, visit http://gerrit.cloudera.org:8080/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 21:30:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11102 ) Change subject: IMPALA-7317: loosen flake8 rules .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/139/ : 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/11102 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997 Gerrit-Change-Number: 11102 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Brown Gerrit-Comment-Date: Wed, 01 Aug 2018 21:28:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7297: soft memory limit for reservation increases
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/10988 ) Change subject: IMPALA-7297: soft memory limit for reservation increases .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39dcf2dd870a59c8b8cda4488fe41ce936d715ac Gerrit-Change-Number: 10988 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 21:05:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7317: loosen flake8 rules
Tim Armstrong has uploaded this change for review. (
http://gerrit.cloudera.org:8080/11102
Change subject: IMPALA-7317: loosen flake8 rules
..
IMPALA-7317: loosen flake8 rules
I got feedback that the flake8 rules are a little too strict, so based
on the d...@impala.apache.org discussion am disabling some of them.
I left E302 "expected 2 blank lines, found 1" enabled since I think it
would force our formatting to be more consistent, but I'm open to
changing that.
I used the following command to get a sense of the number of existing
violations:
EXCLUDES=ext-py,shell/build,gerrit_critic_venv,infra/python/env
EXCLUDES+=,shell/pkg_resources.py,shell/thrift_sasl.py,gen-py
EXCLUDES+=,TPC-DS-master,toolchain
./bin/impala-flake8 --exclude=${EXCLUDES}
Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
---
M bin/jenkins/critique-gerrit-review.py
M setup.cfg
2 files changed, 7 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/11102/1
--
To view, visit http://gerrit.cloudera.org:8080/11102
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaaa8377025e732231ac0f1f1f2db67298a171997
Gerrit-Change-Number: 11102
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py@42 PS5, Line 42: def find_query_option(key, string, strip_brackets=True): > Are we sure that we plan to follow this rule in Impala? I keep it as it is We were having a discussion about this on the mailing list. I'm actually in favour of enabling this one although a lot of existing code doesn't follow it just for the sake of consistency -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 20:46:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5839: [DOCS] Corrected the return types for NULLIFZERO and NULLIFZERO
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/11024 ) Change subject: IMPALA-5839: [DOCS] Corrected the return types for NULLIFZERO and NULLIFZERO .. Patch Set 2: Hi Phil, Could you review this short change? Or if you recommend another reviewer, please add the reviewer. Thanks! -- To view, visit http://gerrit.cloudera.org:8080/11024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7927a5caf1d200a1f4d04d3729c29391c7f43cd Gerrit-Change-Number: 11024 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Philip Zeyliger Gerrit-Comment-Date: Wed, 01 Aug 2018 20:06:49 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Added the part 1 of IMPALA-5607 to the upgrade guide
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/11051 ) Change subject: [DOCS] Added the part 1 of IMPALA-5607 to the upgrade guide .. Patch Set 1: Sorry for the messy diffs. You can review from L363 for the actual change. -- To view, visit http://gerrit.cloudera.org:8080/11051 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib0e53959bef4c629a31868e16b03b6abc11c9f8d Gerrit-Change-Number: 11051 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jinchul Kim Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 20:04:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/138/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:58:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6335. Allow most shell tests to run in parallel
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11045 ) Change subject: IMPALA-6335. Allow most shell tests to run in parallel .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1da5739276e63a50590dfcb2b050703f8e35fec7 Gerrit-Change-Number: 11045 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 19:53:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/137/ : 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/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:45:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7213: 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: Port ReportExecStatus() RPC to use KRPC .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/136/ : 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: 5 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 19:37:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py File tests/custom_cluster/test_local_tz_conversion.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py@53 PS5, Line 53: " > flake8: E501 line too long (91 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py@42 PS5, Line 42: def find_query_option(key, string, strip_brackets=True): > flake8: E302 expected 2 blank lines, found 1 Are we sure that we plan to follow this rule in Impala? I keep it as it is for now as there was only 1 line before the change. -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:29:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/11064/6/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/6/tests/shell/test_shell_commandline.py@42 PS6, Line 42: def find_query_option(key, string, strip_brackets=True): flake8: E302 expected 2 blank lines, found 1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:28:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#6). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 139 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/6 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 6 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Csaba Ringhofer has posted comments on this change. (
http://gerrit.cloudera.org:8080/11064 )
Change subject: IMPALA-7362: Add query option to set timezone
..
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11064/4/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:
http://gerrit.cloudera.org:8080/#/c/11064/4/be/src/service/impala-server.cc@1287
PS4, Line 1287: TimezoneDatabase::LocalZoneName();
> Shouldn't we check if (TimezoneDatabase::FindTimezone(timezone) == nullptr)
I think that we shouldn't worry too much about that case here. I prefer to do
the fallback when starting queries because it gives a nice error message to the
user.
Note that I plan to turn this case to a fatal error during startup in the
future when I will implement IMPALA-7365: Cache local timezone at startup.
http://gerrit.cloudera.org:8080/#/c/11064/4/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
http://gerrit.cloudera.org:8080/#/c/11064/4/common/thrift/ImpalaInternalService.thrift@430
PS4, Line 430: TimezoneDb
> TimezoneDatabase::LocalZoneName()
Done
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test
File testdata/workloads/functional-query/queries/QueryTest/set.test:
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test@237
PS4, Line 237: 1970-01-01 01:00:00
> I'm a little confused. We should get these resutls only if implad is run wi
Ouch, I have added these a bit mindlessly. The tests worked on my local
machine, because the cluster was started with
-use_local_tz_for_unix_timestamp_conversions=true. Thanks for spotting it!
http://gerrit.cloudera.org:8080/#/c/11064/4/testdata/workloads/functional-query/queries/QueryTest/set.test@245
PS4, Line 245: 1969-12-31 16:00:00
> Sme as above
Done
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@42
PS4, Line 42: =
> nit: remove spaces around '='
I think that this function can be useful in other tests too to check the value
of query options, and some may want to keep the brackets to check if the value
is the default or not.
The reason why I prefer this function to assert "X" in result.stdout is that
the stdout is often trimmed in the test's output so the actual value is often
missing from the logs.
+ I have removed the spaces.
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@50
PS4, Line 50: pattern = key + ": .*$"
: values = [s.split(":")[1][1:] for s in re.findall(pattern,
string, re.MULTILINE)]
> This can be simplified a bit:
Done
http://gerrit.cloudera.org:8080/#/c/11064/4/tests/shell/test_shell_commandline.py@714
PS4, Line 714:
:It would be nice to check that the default timezone is
the system's timezone,
:but doing this reliably on different Linux distributions
is quite hard.
> Maybe you could use 'dateutil.tz' module:
I tried to read the exact timestamps in older versions of this review, see
https://gerrit.cloudera.org/#/c/11064/1..4/tests/shell/test_shell_commandline.py
The solution should have worked on Ubuntu, and was ok on my local machine, but
the jenkins job failed with "UTC" vs "Etc/UTC". Probably the difference was
related to following symlinks differently, but I did want to dive in to this
for the sake of the test.
--
To view, visit http://gerrit.cloudera.org:8080/11064
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272
Gerrit-Change-Number: 11064
Gerrit-PatchSet: 4
Gerrit-Owner: Csaba Ringhofer
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 01 Aug 2018 19:25:57 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7297: soft memory limit for reservation increases
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10988 ) Change subject: IMPALA-7297: soft memory limit for reservation increases .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/135/ : 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/10988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39dcf2dd870a59c8b8cda4488fe41ce936d715ac Gerrit-Change-Number: 10988 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 19:19:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11064 ) Change subject: IMPALA-7362: Add query option to set timezone .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py File tests/custom_cluster/test_local_tz_conversion.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/custom_cluster/test_local_tz_conversion.py@53 PS5, Line 53: " flake8: E501 line too long (91 > 90 characters) http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py File tests/shell/test_shell_commandline.py: http://gerrit.cloudera.org:8080/#/c/11064/5/tests/shell/test_shell_commandline.py@42 PS5, Line 42: def find_query_option(key, string, strip_brackets=True): flake8: E302 expected 2 blank lines, found 1 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 19:06:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7362: Add query option to set timezone
Hello Attila Jeges, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11064 to look at the new patch set (#5). Change subject: IMPALA-7362: Add query option to set timezone .. IMPALA-7362: Add query option to set timezone This change adds a new query option "timezone" which defines the timezone used for utc<->local conversions. The main goal is to simplify testing, but I think that some users may also find it useful so it is added as a "general" query option. Examples: set timezone=UTC; set timezone="Europe/Budapest" The timezones are validated, but as query options are not sent to the coordinator immediately, the error checking will only happen when running a query. Leading/trailing " and 'characters are stripped because the / character cannot be entered unquoted in some contexts. Currently the timezone has effect in the following cases: -function now() -conversions between unix time and timestamp if flag use_local_tz_for_unix_timestamp_conversions is true -reading parquet timestamps written by Hive if flag convert_legacy_hive_parquet_utc_timestamps is true In the near future Parquet timestamps's isAdjustedToUTC property will be supported, which will decide whether to do utc->local conversion on a per file+column basis. This conversion will be also affected. Testing: - Extended test_local_tz_conversion.py to actually test utc<->local conversion. Until now the effect of flag use_local_tz_for_unix_timestamp_conversions was practically untested. - Added a shell test to check that the default of the query option is the system's timezone. - Added a shell test to check timezone validation. Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 --- M be/src/service/impala-server.cc M be/src/service/query-options.cc M be/src/service/query-options.h M common/thrift/ImpalaInternalService.thrift M common/thrift/ImpalaService.thrift A testdata/workloads/functional-query/queries/QueryTest/local-timestamp-functions.test M tests/custom_cluster/test_local_tz_conversion.py M tests/shell/test_shell_commandline.py M tests/shell/test_shell_interactive.py 9 files changed, 138 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/64/11064/5 -- To view, visit http://gerrit.cloudera.org:8080/11064 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I73de86eff096e1c581d3b56a0d9330d686f77272 Gerrit-Change-Number: 11064 Gerrit-PatchSet: 5 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7213: Port ReportExecStatus() RPC to use KRPC
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/10855 )
Change subject: IMPALA-7213: Port ReportExecStatus() RPC to use KRPC
..
Patch Set 4:
(30 comments)
http://gerrit.cloudera.org:8080/#/c/10855/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10855/4//COMMIT_MSG@16
PS4, Line 16: 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.
> yep, that's essentially what I meant. Thanks for explaining it better than
Thanks for the suggestion. I think it's fine to do it as a follow-up or
parallel work on Kudu side. This doesn't necessarily block the merging of this
patch IMHO as ReportExecStatus() can already be late today for various reasons
such as an overloaded coordinator.
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/coordinator.h@31
PS4, Line 31: #include "kudu/util/slice.h"
> Not needed?
Done
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/dml-exec-state.cc
File be/src/runtime/dml-exec-state.cc:
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/dml-exec-state.cc@404
PS4, Line 404: void DmlExecState::ToProto(InsertExecStatusPB* dml_status) {
> do you want to clear dml_status first just in case? Similarly for other ToP
Done
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/dml-exec-state.cc@455
PS4, Line 455:if (!kudu_stats->has_num_row_errors()) {
: kudu_stats->set_num_row_errors(0);
: }
> I don't think this is necessary- it's not illegal to access an "unset" fiel
Done
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/exec-env.h
File be/src/runtime/exec-env.h:
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/exec-env.h@136
PS4, Line 136: DataStreamService* data_svc() const { return data_svc_.get(); }
> Not used, we can remove this.
Done
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@268
PS4, Line 268: rpc_controller.AddOutboundSidecar(move(sidecar), _idx).ok
> should this be a CHECK_OK?
Done
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@273
PS4, Line 273: "final"
> need a space here
Oops. Done.
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@274
PS4, Line 274: ERROR
> should this be a DFATAL? do you expect this to ever actually happen?
I don't think this is expected to happen very often but it seems more robust to
not crash Impala because of failure to serialize the query profile. The query
can still run to completion without the profile.
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@283
PS4, Line 283: req.clear_error_log();
> isnt the error log already clear because this is a new instance?
Done
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@284
PS4, Line 284: state->GetUnreportedErrors(req.mutable_error_log());
> it's a shame that this mutates the state, making retries a bit more difficu
The new patch creates a single instance of the RPC parameter and reuse it on
retry. My understanding is that the RPC layer shouldn't mutate the input RPC
request parameter.
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@289
PS4, Line 289: rpc_mgr->GetProxy
> clang-tidy failure: Missing status check.
Done
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@293
PS4, Line 293: kudu::Status rpc_status = proxy->ReportExecStatus(req, ,
_controller);
> do you want any timeout on this RPC?
Yes. Switched back to the behavior of existing code of using
FLAGS_backend_client_rpc_timeout_ms. This is not ideal but I guess it's
simplest to just preserve the existing behavior and fix IMPALA-2990 in a
separate patch.
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@305
PS4, Line 305: // We can retry the RPC if the payload hasn't been sent yet.
> are these reports idempotent? seems like it shoudl be easy to make them ide
Yes, I believe they are idempotent. So, I was worried about the connection
being reset in the middle of transmission so only partial payload was sent. In
that case, I think the server side will also drop that connection so it should
be fine I suppose. Comments removed.
http://gerrit.cloudera.org:8080/#/c/10855/4/be/src/runtime/query-state.cc@306
PS4, Line 306: RpcMgr::IsServerTooBusy(rpc_controller
> I'm a bit fuzzy on the context of this code, but if we get TOO_BUSY, it see
Restored to the old behavior of retrying for at most 2 times. It will
[Impala-ASF-CR] IMPALA-7213: Port ReportExecStatus() RPC to use KRPC
Hello Sailesh Mukil, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10855 to look at the new patch set (#5). Change subject: IMPALA-7213: Port ReportExecStatus() RPC to use KRPC .. IMPALA-7213: 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. Change-Id: I7638583b433dcac066b87198e448743d90415ebe --- M be/src/benchmarks/expr-benchmark.cc M be/src/catalog/catalog-util.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/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 bin/impala-config.sh 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 58 files changed, 1,050 insertions(+), 634 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/10855/5 -- 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: 5 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7297: soft memory limit for reservation increases
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10988 ) Change subject: IMPALA-7297: soft memory limit for reservation increases .. Patch Set 5: (4 comments) Sorry for the delay, lost track of this one http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/bufferpool/buffer-pool.h File be/src/runtime/bufferpool/buffer-pool.h: http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/bufferpool/buffer-pool.h@178 PS5, Line 178: check_soft_mem_limit > nit: mem_limit_mode Done http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/bufferpool/reservation-tracker.cc File be/src/runtime/bufferpool/reservation-tracker.cc: http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/bufferpool/reservation-tracker.cc@84 PS5, Line 84: MemLimit::HARD > I'm not too familiar with the ReservationTracker stuff, so maybe this is wr I think either way is equivalent, but mem_limit_mode_ probably makes more intuitive sense. http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/mem-tracker-types.h File be/src/runtime/mem-tracker-types.h: http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/mem-tracker-types.h@18 PS5, Line 18: : #ifndef IMPALA_RUNTIME_MEM_TRACKER_TYPES_H : #define IMPALA_RUNTIME_MEM_TRACKER_TYPES_H : > we've moved to '#pragma once' in Kudu for this, no such preference in impal I don't think it's been on our radar. Technically the Google C++ style guide seems to not allow it, but probably it makes sense - I think all our compilers support it. I'll start a discussion on the mailing list and update our style guide on the wiki. https://google.github.io/styleguide/cppguide.html#Windows_Code http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/mem-tracker.cc File be/src/runtime/mem-tracker.cc: http://gerrit.cloudera.org:8080/#/c/10988/5/be/src/runtime/mem-tracker.cc@41 PS5, Line 41: DEFINE_double > maybe make this hidden if we dont expect users to configure it initially? Done -- To view, visit http://gerrit.cloudera.org:8080/10988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I39dcf2dd870a59c8b8cda4488fe41ce936d715ac Gerrit-Change-Number: 10988 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 01 Aug 2018 18:46:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7297: soft memory limit for reservation increases
Hello Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10988 to look at the new patch set (#6). Change subject: IMPALA-7297: soft memory limit for reservation increases .. IMPALA-7297: soft memory limit for reservation increases https://goo.gl/N9LgQt summarises the memory problems I'm trying to solve here. Reject reservation increases that would leave < 10% of the memory available under a query or process mem_limit, which put the query or impalad into a state with a heightened chance of OOM. This avoids OOM in some scenarios where reserved memory is increased too aggressively and starves non-reserved memory. One way of thinking about this that it's extending the 80% limit heuristic for reserved memory so that there's a soft limit of min(80%, 90% - non-reserved memory consumption) A follow-on will use this for scanner threads to prevent starting scanner threads that may exceed the soft limit. Testing: Added unit tests for MemTracker and for ReservationTracker. Change-Id: I39dcf2dd870a59c8b8cda4488fe41ce936d715ac --- M be/src/exec/exchange-node.cc M be/src/rpc/impala-service-pool.cc M be/src/runtime/bufferpool/buffer-pool-internal.h M be/src/runtime/bufferpool/buffer-pool.cc M be/src/runtime/bufferpool/buffer-pool.h M be/src/runtime/bufferpool/reservation-tracker-test.cc M be/src/runtime/bufferpool/reservation-tracker.cc M be/src/runtime/bufferpool/reservation-tracker.h M be/src/runtime/initial-reservations.cc M be/src/runtime/mem-pool-test.cc M be/src/runtime/mem-tracker-test.cc A be/src/runtime/mem-tracker-types.h M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h M be/src/runtime/query-state.cc M be/src/runtime/runtime-state.cc M be/src/udf/udf.cc 17 files changed, 313 insertions(+), 151 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/10988/6 -- To view, visit http://gerrit.cloudera.org:8080/10988 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I39dcf2dd870a59c8b8cda4488fe41ce936d715ac Gerrit-Change-Number: 10988 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7381: Prevent build failure after switching to new CDH BUILD NUMBER
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/11099 )
Change subject: IMPALA-7381: Prevent build failure after switching to new
CDH_BUILD_NUMBER
..
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11099/1/bin/clean.sh
File bin/clean.sh:
http://gerrit.cloudera.org:8080/#/c/11099/1/bin/clean.sh@81
PS1, Line 81: rm -rf ${IMPALA_HOME}/toolchain
> We need to be careful here, because I think it is useful to be able to deve
Removing toolchain/cdh_components is necessary to make sure the CDH components
stay consistent with their Maven dependencies. For now, we can probably just
delete toolchain/cdh_components and decide if in the future we need to also
delete the whole toolchain directory? We can also push $CDH_BUILD_NUMBER into
cdh_components but that may require more changes in other places. I'm okay
either way though.
mvn -U is something that Todd proposed in the mailing list that seems to work.
It's pretty annoying that every time we bump the CDH_BUILD_NUMBER, we need to
tell everyone to delete toolchain/cdh_components and run mvn -U to force the
Maven cache and not to mention occasional build failures in Jenkins when the
Maven cache isn't up-to-date. There's still another issue such as stale
environment variables that we need to solve, but that's for another time I
guess :)
For development, I believe most people run their build with buildall.sh
-noclean. So, the slow build shouldn't impact them. The changes should only
impact when people do a clean build.
--
To view, visit http://gerrit.cloudera.org:8080/11099
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0ad9c2258663d3bd7470e6df921041d1ca0c0be
Gerrit-Change-Number: 11099
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Philip Zeyliger
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Wed, 01 Aug 2018 18:19:06 +
Gerrit-HasComments: Yes
[native-toolchain-CR] IMPALA-7364: Add RapidJson 1.1.0
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11092 ) Change subject: IMPALA-7364: Add RapidJson 1.1.0 .. Patch Set 1: Verified+1 Was able to confirm this builds ok on all the distros I have access to. -- To view, visit http://gerrit.cloudera.org:8080/11092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie98462fde43c97101e95d0761be1594bc2636859 Gerrit-Change-Number: 11092 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 18:15:10 + Gerrit-HasComments: No
[native-toolchain-CR] IMPALA-7364: Add RapidJson 1.1.0
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11092 ) Change subject: IMPALA-7364: Add RapidJson 1.1.0 .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/11092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie98462fde43c97101e95d0761be1594bc2636859 Gerrit-Change-Number: 11092 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 18:15:17 + Gerrit-HasComments: No
[native-toolchain-CR] Add RapidJson 1.1.0
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11092 ) Change subject: Add RapidJson 1.1.0 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11092/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11092/1//COMMIT_MSG@7 PS1, Line 7: Add RapidJson 1.1.0 > Reference IMPALA-7364? Done -- To view, visit http://gerrit.cloudera.org:8080/11092 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: native-toolchain Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie98462fde43c97101e95d0761be1594bc2636859 Gerrit-Change-Number: 11092 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 01 Aug 2018 18:14:40 + Gerrit-HasComments: Yes
