[Impala-ASF-CR] IMPALA-6625: Skip computing parquet conjuncts for non-Parquet scans
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10704 ) Change subject: IMPALA-6625: Skip computing parquet conjuncts for non-Parquet scans .. Patch Set 14: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2772/ -- To view, visit http://gerrit.cloudera.org:8080/10704 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d6c26d42db090c8a15c602f6419ad6399c329e7 Gerrit-Change-Number: 10704 Gerrit-PatchSet: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 04 Jul 2018 04:46:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10863 ) Change subject: IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10863/1/be/src/exec/grouping-aggregator.cc File be/src/exec/grouping-aggregator.cc: http://gerrit.cloudera.org:8080/#/c/10863/1/be/src/exec/grouping-aggregator.cc@407 PS1, Line 407: SCOPED_TIMER(build_timer_); Do we have a similar bug here? I'm not sure how expr_results_pool_ gets cleared in the aggregator when driven from Open(). Same for the non-grouping aggregator. It would be nice to avoid the need for these two sets of QueryMaintenance() calls to clear expr_results_pool_ at both levels. We could consider using ExecNode::expr_results_pool_, which reduces the granularity of some of the memory tracking but I think should be ok - the "result" allocations are expected to be small unless an expr misbehaves. Otherwise we could have the parent ExecNode() clear the results pools on the aggregators at the same points as it's clearing its own pools to avoid this duplication. -- To view, visit http://gerrit.cloudera.org:8080/10863 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I150a46d00acad139d186a150d9706ef47a10ac36 Gerrit-Change-Number: 10863 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 04 Jul 2018 02:37:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10863 ) Change subject: IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10863/1/be/src/exec/grouping-aggregator.cc File be/src/exec/grouping-aggregator.cc: http://gerrit.cloudera.org:8080/#/c/10863/1/be/src/exec/grouping-aggregator.cc@423 PS1, Line 423: RETURN_IF_ERROR(QueryMaintenance(state)); expr_results_pool_ is used in AddBatchStreamingImpl(), right ? -- To view, visit http://gerrit.cloudera.org:8080/10863 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I150a46d00acad139d186a150d9706ef47a10ac36 Gerrit-Change-Number: 10863 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 04 Jul 2018 02:01:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10863 ) Change subject: IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10863 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I150a46d00acad139d186a150d9706ef47a10ac36 Gerrit-Change-Number: 10863 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 04 Jul 2018 02:01:49 + Gerrit-HasComments: No
[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 4:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-exec-mgr.cc
File be/src/runtime/query-exec-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-exec-mgr.cc@128
PS4, Line 128: qs->MonitorFInstances();
This seems to fit into IMPALA-4063 better. As the code stands in this patch, we
are unnecessarily blocking this thread from exiting after starting all fragment
instances. It makes sense in IMPALA-4063 as this is the thread responsible for
aggregating and sending the profiles for all fragment instances.
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@159
PS4, Line 159: IMPALA-2990
IMPALA-4063
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@278
PS4, Line 278: CANCELLED,
No implication on whether all fragment instances have finished cancelling
themselves, right ?
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@292
PS4, Line 292: IMPALA-2990
Did you mean IMPALA-4063 ? Does it mean this comment needs to be removed. May
be convert the stuff which needs to be added in IMPALA-4063 as a TODO instead
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@350
PS4, Line 350: boost::scoped_ptr
std::unique_ptr
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.h@355
PS4, Line 355: boost::scoped_ptr
std::unique_ptr
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@a350
PS4, Line 350:
:
Was this a bug ?! Will we hang in the for loop at line 363 below if we fail to
create a thread ?
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@141
PS4, Line 141: instances_prepared_barrier_
As discussed offline, the behavior during "preparation" phase of the query is
that we will wait for fragment instances to finish preparing even if some of
them run into errors. So, in that sense, the existing code seems to suffice
unless I misunderstood something.
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@225
PS4, Line 225: DCHECK(true)
DCHECK(false) ?! Does this warrant a targeted BE test ?
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@230
PS4, Line 230: QueryState::HandleExecStateTransition
This function seems like a no-op in this patch. Can this be added in the next
patch when necessary ?
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@278
PS4, Line 278: if (!status.ok() && (!status.IsCancelled() ||
!IsTerminalState(old_state))) {
: VLOG_QUERY << Substitute("BackendExecState: query id=$0
finstance=$1 ($2 -> $3) "
: "status=$4",
: PrintId(query_id()),
: failed_instance != TUniqueId() ?
PrintId(failed_instance) : "N/A",
: BackendExecStateToString(old_state),
BackendExecStateToString(new_state),
: status.GetDetail());
Will this actually be duplicating the error message which we usually log
already when there is an error ?
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@391
PS4, Line 391: return WaitForPrepareInternal().status_;
As discussed offline, there is no need to have a different implementation for
waiting for all fis to finish preparing unless we can simplify it further
somehow.
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@434
PS4, Line 434: IMPALA-2990
IMPALA-4063 ?
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@502
PS4, Line 502: QueryState::MonitorFInstances()
Please see comments in query-exec-mgr.cc
http://gerrit.cloudera.org:8080/#/c/10813/4/be/src/runtime/query-state.cc@542
PS4, Line 542: // initiate cancellation if nobody has done so yet
: if (!status.ok()) Cancel();
Not sure what you think about the following. It appears to me that we may not
necessarily need to UpdateBackendState() above to track the explicit state
switching. Instead we can do something like below in this function and some add
on to Cancel():
if (!status.ok()) {
Cancel();
instances_finished_barrier_->NotifyRemaining(status);
} else {
instances_finished_barrier_->Notify();
}
So, in IMPALA-4063, the query state thread will wait on this
instances_finished_barrier_ after all fragment instances are done preparing.
Once the wait returns, it may consider sending the profile right away
[Impala-ASF-CR] IMPALA-3040: Remove cache directives if a partition is dropped externally
Tianyi Wang has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/10792 ) Change subject: IMPALA-3040: Remove cache directives if a partition is dropped externally .. IMPALA-3040: Remove cache directives if a partition is dropped externally HdfsTable.dropPartition() doesn't uncache the partition right now. If the partition is dropped from Hive and refreshed in Impala, the partition will be removed from the catalog but the cache directive remains. Because Impala directly uses HMS client to drop a table/database, the cache directive won't be removed even if the table is dropped in Impala, if the backgroud loading is run concurrenty with the HMS client RPC call. This patch removes the cache directive in dropPartition() to fix this bug. Change-Id: Id7701a499405e961456adea63f3592b43bd69170 --- M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/query_test/test_hdfs_caching.py 3 files changed, 28 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/10792/4 -- To view, visit http://gerrit.cloudera.org:8080/10792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id7701a499405e961456adea63f3592b43bd69170 Gerrit-Change-Number: 10792 Gerrit-PatchSet: 4 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-3040: Remove cache directives if a partition is dropped externally
Tianyi Wang has posted comments on this change. ( http://gerrit.cloudera.org:8080/10792 ) Change subject: IMPALA-3040: Remove cache directives if a partition is dropped externally .. Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG@7 PS3, Line 7: if a partition is dropped externally > may be say external table drops? Done http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG@9 PS3, Line 9: > Add some context about what happens when it is dropped from Hive. Done http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG@13 PS3, Line 13: table/database, the cache directive won't be removed even if the table > Could you add a test for this in test_hdfs_caching? Done http://gerrit.cloudera.org:8080/#/c/10792/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/10792/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1158 PS3, Line 1158:* HdfsPartition that was dropped or null if the partition does not exist. > Update that this drops the cache directive if its cached. Done http://gerrit.cloudera.org:8080/#/c/10792/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1191 PS3, Line 1191: // If there are multiple partition ids corresponding to a literal, remove > I think this should only run on the Catalog server? I checked the callers of this function and it can only be called from catalogd. http://gerrit.cloudera.org:8080/#/c/10792/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1193 PS3, Line 1193: if (partitionIds.size() > 1) partitionIds.remove(partitionId); > Do we need to remove a similar check from CatalogOpEx#alterTableDropPartiti Done -- To view, visit http://gerrit.cloudera.org:8080/10792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7701a499405e961456adea63f3592b43bd69170 Gerrit-Change-Number: 10792 Gerrit-PatchSet: 4 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Tianyi Wang Gerrit-Comment-Date: Wed, 04 Jul 2018 00:54:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10863 Change subject: IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming .. IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming A recent change, IMPALA-110 (part 2), refactored the aggregation code. During this refactor, a call to QueryMaintenance() was inadvertantely dropped in the AddBatchStreaming() path, causing test_spilling_regression_exhaustive to fail by hitting the memory limit. Testing: - Ran test_spilling_regression_exhaustive Change-Id: I150a46d00acad139d186a150d9706ef47a10ac36 --- M be/src/exec/grouping-aggregator.cc 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/10863/1 -- To view, visit http://gerrit.cloudera.org:8080/10863 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I150a46d00acad139d186a150d9706ef47a10ac36 Gerrit-Change-Number: 10863 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10857 ) Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Comment-Date: Tue, 03 Jul 2018 23:49:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10857 ) Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES This patch adds a missing "break" statement in a switch statement changed by IMPALA-7102. Also fixes an non-deterministic test case. Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Reviewed-on: http://gerrit.cloudera.org:8080/10857 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/service/query-options.cc M testdata/workloads/functional-query/queries/QueryTest/hdfs-erasure-coding.test 2 files changed, 2 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 3 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-6034: Add CPU and scanned bytes limits per query
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10415 ) Change subject: IMPALA-6034: Add CPU and scanned bytes limits per query .. Patch Set 6: (6 comments) http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.h@198 PS8, Line 198: > For this fragment instance Done http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.h@207 PS8, Line 207: /// Collection of BYTES_READ_COUNTERs of all scan nodes in this fragment instance. > peak_mem_counter_ was removed... Done http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.cc@294 PS8, Line 294: // TODO: We're losing this profile information. Call ReportQuerySummary only after > I'm considering adjusting this to use an O(1) algorithm - Update() could re Done http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator.h@177 PS8, Line 177: > This isn't accurate, could be fragment instances, backend or query. Done http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.h@135 PS8, Line 135:TQueryOptionLevel::ADVANCED)\ > Need to fix tabs Done http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.cc@682 PS8, Line 682: // Parse the scan bytes limit and validate it. > Useless comment. Done -- To view, visit http://gerrit.cloudera.org:8080/10415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20 Gerrit-Change-Number: 10415 Gerrit-PatchSet: 6 Gerrit-Owner: Mostafa Mokhtar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 23:40:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7238: Use custom timeout for create unique database
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/10862 ) Change subject: IMPALA-7238: Use custom timeout for create unique database .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10862/1/tests/conftest.py File tests/conftest.py: http://gerrit.cloudera.org:8080/#/c/10862/1/tests/conftest.py@391 PS1, Line 391: with __auto_closed_conn(timeout=timeout) as conn: Another way to fix it is to use a higher timeout universally. -- To view, visit http://gerrit.cloudera.org:8080/10862 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4f2beb5bc027a4bb44e854bf1dd8919807a92ea0 Gerrit-Change-Number: 10862 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 03 Jul 2018 23:30:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7238: Use custom timeout for create unique database
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10862 Change subject: IMPALA-7238: Use custom timeout for create unique database .. IMPALA-7238: Use custom timeout for create unique database test_kudu.TestCreateExternalTables() saw a timeout when creating the unique database for its tests. __unique_conn() opens a connection, creates a unique database, then returns another connection in that database. It takes a custom timeout argument, but the timeout is only for the returned connection. The first connection to create the unique database uses the default timeout of 45 seconds. This patch changes the first connection to use the custom timeout. For Kudu tests, this is 5 minutes rather than 45 seconds. Change-Id: I4f2beb5bc027a4bb44e854bf1dd8919807a92ea0 --- M tests/conftest.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/10862/1 -- To view, visit http://gerrit.cloudera.org:8080/10862 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I4f2beb5bc027a4bb44e854bf1dd8919807a92ea0 Gerrit-Change-Number: 10862 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell
[Impala-ASF-CR] IMPALA-6034: Add CPU and scanned bytes limits per query
Tim Armstrong has uploaded a new patch set (#9) to the change originally created by Mostafa Mokhtar. ( http://gerrit.cloudera.org:8080/10415 ) Change subject: IMPALA-6034: Add CPU and scanned bytes limits per query .. IMPALA-6034: Add CPU and scanned bytes limits per query To protect Impala from runaway queries add per query limits for CPU and scanned bytes, limits are enforced via query options and applied to the entire query, not per backend like mem_limit. If a query exceeds any of the limits it will get cancelled. Query profile is updated to include query wide and per backend metrics for Cpu and scanned bytes. Testing: Added tests for various permutations for CPU_LIMIT_S and SCAN_BYTES_LIMIT Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20 --- 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/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options-test.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/query-resource-limits.test M tests/query_test/test_cancellation.py M tests/query_test/test_resource_limits.py 14 files changed, 351 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/10415/9 -- To view, visit http://gerrit.cloudera.org:8080/10415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20 Gerrit-Change-Number: 10415 Gerrit-PatchSet: 9 Gerrit-Owner: Mostafa Mokhtar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-3956: [DOCS] Escape variables with '\' in impala-shell
Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10861 Change subject: IMPALA-3956: [DOCS] Escape variables with '\' in impala-shell .. IMPALA-3956: [DOCS] Escape variables with '\' in impala-shell Change-Id: Ifb95785a143939a94d55d3565364afe1e26c1f3d --- M docs/topics/impala_shell_commands.xml M docs/topics/impala_shell_running_commands.xml 2 files changed, 125 insertions(+), 299 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/10861/1 -- To view, visit http://gerrit.cloudera.org:8080/10861 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ifb95785a143939a94d55d3565364afe1e26c1f3d Gerrit-Change-Number: 10861 Gerrit-PatchSet: 1 Gerrit-Owner: Alex Rodoni
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 6 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Jul 2018 23:21:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. IMPALA-6883: [DOCS] Refactor impala_authorization doc Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Reviewed-on: http://gerrit.cloudera.org:8080/10786 Reviewed-by: Alex Rodoni Tested-by: Impala Public Jenkins --- M docs/shared/impala_common.xml M docs/topics/impala_authorization.xml M docs/topics/impala_grant.xml 3 files changed, 543 insertions(+), 701 deletions(-) Approvals: Alex Rodoni: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 7 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/339/ -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 6 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Jul 2018 23:12:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 6 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Jul 2018 23:11:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10786 to look at the new patch set (#6). Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. IMPALA-6883: [DOCS] Refactor impala_authorization doc Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d --- M docs/shared/impala_common.xml M docs/topics/impala_authorization.xml M docs/topics/impala_grant.xml 3 files changed, 543 insertions(+), 701 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10786/6 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 6 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-3040: Remove cache directives during background partition dropping
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/10792 )
Change subject: IMPALA-3040: Remove cache directives during background
partition dropping
..
Patch Set 3:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG@7
PS3, Line 7: during background partition dropping
may be say external table drops?
http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG@9
PS3, Line 9:
Add some context about what happens when it is dropped from Hive.
http://gerrit.cloudera.org:8080/#/c/10792/3//COMMIT_MSG@13
PS3, Line 13:
Could you add a test for this in test_hdfs_caching?
http://gerrit.cloudera.org:8080/#/c/10792/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/10792/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1158
PS3, Line 1158:* HdfsPartition that was dropped or null if the partition
does not exist.
Update that this drops the cache directive if its cached.
http://gerrit.cloudera.org:8080/#/c/10792/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1191
PS3, Line 1191: if (partition.isMarkedCached()) {
I think this should only run on the Catalog server?
http://gerrit.cloudera.org:8080/#/c/10792/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1193
PS3, Line 1193:
HdfsCachingUtil.removePartitionCacheDirective(partition);
Do we need to remove a similar check from
CatalogOpEx#alterTableDropPartition()? No point in doing it twice.
--
To view, visit http://gerrit.cloudera.org:8080/10792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7701a499405e961456adea63f3592b43bd69170
Gerrit-Change-Number: 10792
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Tianyi Wang
Gerrit-Comment-Date: Tue, 03 Jul 2018 22:39:48 +
Gerrit-HasComments: Yes
[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 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/10855/2/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: http://gerrit.cloudera.org:8080/#/c/10855/2/be/src/runtime/query-state.cc@263 PS2, Line 263: serialize_status.ok() Need a test case for serialization failure. May be global debug action is useful ? http://gerrit.cloudera.org:8080/#/c/10855/2/be/src/runtime/query-state.cc@303 PS2, Line 303: !RpcMgr::IsServerTooBusy(rpc_controller)) Need a test case for this. http://gerrit.cloudera.org:8080/#/c/10855/2/be/src/service/control-service.cc File be/src/service/control-service.cc: http://gerrit.cloudera.org:8080/#/c/10855/2/be/src/service/control-service.cc@107 PS2, Line 107: (UNLIKELY(!deserialize_status.ok()) Need a test case for deserialization error. -- 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: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil Gerrit-Comment-Date: Tue, 03 Jul 2018 22:30:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3040: Remove cache directives during background partition dropping
Tianyi Wang has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/10792 ) Change subject: IMPALA-3040: Remove cache directives during background partition dropping .. IMPALA-3040: Remove cache directives during background partition dropping HdfsTable.dropPartition() doesn't uncache the partition right now. If the table is later dropped, the partition won't be uncached either because it has been removed then. This patch removes the cache directive in dropPartition() to fix this bug. Change-Id: Id7701a499405e961456adea63f3592b43bd69170 --- M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java 1 file changed, 8 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/92/10792/3 -- To view, visit http://gerrit.cloudera.org:8080/10792 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id7701a499405e961456adea63f3592b43bd69170 Gerrit-Change-Number: 10792 Gerrit-PatchSet: 3 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-7202: Add width bucket() to the decimal fuzz test
Taras Bobrovytsky has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10859 Change subject: IMPALA-7202: Add width_bucket() to the decimal fuzz test .. IMPALA-7202: Add width_bucket() to the decimal fuzz test In this patch we include the newly added width_bucket() function in the decimal fuzz test. Change-Id: I1f8a63733ebddc07f46c525ca51ad4794dd0 --- M tests/query_test/test_decimal_fuzz.py 1 file changed, 57 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10859/1 -- To view, visit http://gerrit.cloudera.org:8080/10859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1f8a63733ebddc07f46c525ca51ad4794dd0 Gerrit-Change-Number: 10859 Gerrit-PatchSet: 1 Gerrit-Owner: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/338/ -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Jul 2018 21:14:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10850 ) Change subject: IMPALA-6086: Use of permanent function should require SELECT privilege on DB .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10850 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57 Gerrit-Change-Number: 10850 Gerrit-PatchSet: 3 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 03 Jul 2018 21:04:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB
Zoram Thanga has posted comments on this change. (
http://gerrit.cloudera.org:8080/10850 )
Change subject: IMPALA-6086: Use of permanent function should require SELECT
privilege on DB
..
Patch Set 2:
(6 comments)
Thanks. Please see PS#3.
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2084
PS2, Line 2084:
> nit: add two extra spaces for continued indentation
Done
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2087
PS2, Line 2087: "select functional.to_lower('ABCDEF')")
> nit: move L2088 to L2087
Done
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2091
PS2, Line 2091: .ok(onDatabase("functional", TPrivilegeLevel.ALL))
> nit: move .ok(onDatabase("functional", TPrivilegeLevel.ALL)) to be at the b
Done
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2092
PS2, Line 2092: .ok(onDatabase("functional", TPrivilegeLevel.INSERT))
: .ok(onDatabase("functional",
TPrivilegeLevel.REFRESH))
> INSERT, REFRESH, and SELECT can be simplified to .ok(onDatabase("functional
Done. Want me to wrap ALL under viewMetadataPrivileges() as well?
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2096
PS2, Line 2096: TPrivilegeLevel.SELECT, TPrivilegeLevel.ALL,
: TPrivilegeLevel.INSERT,
TPrivilegeLevel.REFRESH
> can be simplified to allExcept(viewMetadataPrivileges())
Done
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2167
PS2, Line 2167: uriPath, symbolName, null, null,
> nit: move L2168 to L2167
Done
--
To view, visit http://gerrit.cloudera.org:8080/10850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Gerrit-Change-Number: 10850
Gerrit-PatchSet: 2
Gerrit-Owner: Zoram Thanga
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Tue, 03 Jul 2018 20:40:38 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10857 ) Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2773/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Comment-Date: Tue, 03 Jul 2018 20:38:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10857 ) Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 2 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Comment-Date: Tue, 03 Jul 2018 20:38:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7190: Remove unsupported format writer support
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10823 ) Change subject: IMPALA-7190: Remove unsupported format writer support .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I821dc7495a901f1658daa500daf3791b386c7185 Gerrit-Change-Number: 10823 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 20:34:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7190: Remove unsupported format writer support
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10823 ) Change subject: IMPALA-7190: Remove unsupported format writer support .. IMPALA-7190: Remove unsupported format writer support This patch removes write support for unsupported formats like Sequence, Avro and compressed text. Also, the related query options ALLOW_UNSUPPORTED_FORMATS and SEQ_COMPRESSION_MODE have been migrated to the REMOVED query options type. Testing: Ran exhaustive build. Change-Id: I821dc7495a901f1658daa500daf3791b386c7185 Reviewed-on: http://gerrit.cloudera.org:8080/10823 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/CMakeLists.txt D be/src/exec/hdfs-avro-table-writer.cc D be/src/exec/hdfs-avro-table-writer.h D be/src/exec/hdfs-sequence-table-writer.cc D be/src/exec/hdfs-sequence-table-writer.h M be/src/exec/hdfs-table-sink.cc M be/src/exec/hdfs-text-table-writer.cc M be/src/exec/hdfs-text-table-writer.h M be/src/service/query-options-test.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 M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java M testdata/bad_avro_snap/README D testdata/workloads/functional-query/queries/QueryTest/avro-writer.test D testdata/workloads/functional-query/queries/QueryTest/seq-writer.test M testdata/workloads/functional-query/queries/QueryTest/set.test D testdata/workloads/functional-query/queries/QueryTest/text-writer.test A testdata/workloads/functional-query/queries/QueryTest/unsupported-writers.test M tests/common/test_dimensions.py M tests/hs2/test_hs2.py M tests/metadata/test_partition_metadata.py M tests/query_test/test_compressed_formats.py M tests/shell/test_shell_interactive.py 25 files changed, 121 insertions(+), 1,607 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I821dc7495a901f1658daa500daf3791b386c7185 Gerrit-Change-Number: 10823 Gerrit-PatchSet: 5 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/10850 )
Change subject: IMPALA-6086: Use of permanent function should require SELECT
privilege on DB
..
Patch Set 2:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2084
PS2, Line 2084:
nit: add two extra spaces for continued indentation
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2087
PS2, Line 2087: "select functional.to_lower('ABCDEF')")
nit: move L2088 to L2087
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2091
PS2, Line 2091: .ok(onDatabase("functional", TPrivilegeLevel.ALL))
nit: move .ok(onDatabase("functional", TPrivilegeLevel.ALL)) to be at the
beginning (L2089) to be consistent with other code
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2092
PS2, Line 2092: .ok(onDatabase("functional", TPrivilegeLevel.INSERT))
: .ok(onDatabase("functional",
TPrivilegeLevel.REFRESH))
INSERT, REFRESH, and SELECT can be simplified to .ok(onDatabase("functional",
viewMetadataPrivileges())
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2096
PS2, Line 2096: TPrivilegeLevel.SELECT, TPrivilegeLevel.ALL,
: TPrivilegeLevel.INSERT,
TPrivilegeLevel.REFRESH
can be simplified to allExcept(viewMetadataPrivileges())
http://gerrit.cloudera.org:8080/#/c/10850/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2167
PS2, Line 2167: uriPath, symbolName, null, null,
nit: move L2168 to L2167
--
To view, visit http://gerrit.cloudera.org:8080/10850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Gerrit-Change-Number: 10850
Gerrit-PatchSet: 2
Gerrit-Owner: Zoram Thanga
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Tue, 03 Jul 2018 20:15:21 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB
Hello Fredy Wijaya, Vuk Ercegovac, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10850 to look at the new patch set (#2). Change subject: IMPALA-6086: Use of permanent function should require SELECT privilege on DB .. IMPALA-6086: Use of permanent function should require SELECT privilege on DB To use a permanent UDF should require at leat SELECT privilege on the database. Functions that have constant arguments get constant-folded into string literals, losing their privilege requests in the process. This patch saves the privilege requests found during the first phase of query analysis, where all the objects and the privileges required to access them are identified. The requests are added back to the new analyzer created for re-analysis post expression rewrite. New FE test cases have been added to AuthorizationStmtTest. Manual tests were also done to identify the bug, as well as to test the fix. Ran private parameterized Jenkins job, exhaustive exploration and covering all tests. Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57 --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java 2 files changed, 56 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/10850/2 -- To view, visit http://gerrit.cloudera.org:8080/10850 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57 Gerrit-Change-Number: 10850 Gerrit-PatchSet: 2 Gerrit-Owner: Zoram Thanga Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6086: Use of permanent function should require SELECT privilege on DB
Zoram Thanga has posted comments on this change. (
http://gerrit.cloudera.org:8080/10850 )
Change subject: IMPALA-6086: Use of permanent function should require SELECT
privilege on DB
..
Patch Set 1:
(1 comment)
Thanks for the review. Please have a look at PS#2
http://gerrit.cloudera.org:8080/#/c/10850/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/10850/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@2071
PS1, Line 2071: authorize("create function to_lower(string) returns string
location " +
> As mentioned in the previous review, this code doesn't create a function in
Thanks for the education! I've made the suggested changes.
--
To view, visit http://gerrit.cloudera.org:8080/10850
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee70f15e4c04f7daaed9cac2400ec626e1fb0e57
Gerrit-Change-Number: 10850
Gerrit-PatchSet: 1
Gerrit-Owner: Zoram Thanga
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Reviewer: Zoram Thanga
Gerrit-Comment-Date: Tue, 03 Jul 2018 20:02:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7027: Mulitple varchar cast fails with distinct
Adam Holley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10858 Change subject: IMPALA-7027: Mulitple varchar cast fails with distinct .. IMPALA-7027: Mulitple varchar cast fails with distinct When AggregateInfo removes duplicates of groupingExprs during the second pass of rewrites, the CastExprs becomes StringLiterals. Because the localEquals checks only comparase value, which in this case is "" for both, the second cast expr is removed. This causes the second castTo() to fail. This fix adds a check to StringLiteral.localEquals to compare the type so that different items will not be removed. Testing: - Added test to validate distinct with casts - Ran all FE tests - Ran all E2E tests Change-Id: I2fa5890eaa89787645c7d3d2eef976f54a34e7c0 --- M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java 2 files changed, 12 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10858/1 -- To view, visit http://gerrit.cloudera.org:8080/10858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I2fa5890eaa89787645c7d3d2eef976f54a34e7c0 Gerrit-Change-Number: 10858 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley
[Impala-ASF-CR] IMPALA-3956 - Impala shell should ignore variables in comments
Adam Holley has abandoned this change. ( http://gerrit.cloudera.org:8080/10784 ) Change subject: IMPALA-3956 - Impala shell should ignore variables in comments .. Abandoned Changed to doc fix. -- To view, visit http://gerrit.cloudera.org:8080/10784 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I9e442e169d512daf80c86606528300ce4e6f371c Gerrit-Change-Number: 10784 Gerrit-PatchSet: 3 Gerrit-Owner: Adam Holley Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Jul 2018 19:27:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Jul 2018 19:16:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10786 ) Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/336/ -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Tue, 03 Jul 2018 19:17:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6883: [DOCS] Refactor impala authorization doc
Hello Fredy Wijaya, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10786 to look at the new patch set (#5). Change subject: IMPALA-6883: [DOCS] Refactor impala_authorization doc .. IMPALA-6883: [DOCS] Refactor impala_authorization doc Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d --- M docs/shared/impala_common.xml M docs/topics/impala_authorization.xml M docs/topics/impala_grant.xml 3 files changed, 488 insertions(+), 784 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/86/10786/5 -- To view, visit http://gerrit.cloudera.org:8080/10786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3df72adb25dcdcbc286934b048645f47d876b33d Gerrit-Change-Number: 10786 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-6034: Add CPU and scanned bytes limits per query
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10415 ) Change subject: IMPALA-6034: Add CPU and scanned bytes limits per query .. Patch Set 8: (6 comments) There's some more cleanup to do after my first pass... http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.h@198 PS8, Line 198: backend For this fragment instance http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.h@207 PS8, Line 207: /// and peak_mem_counter_ from profile_. peak_mem_counter_ was removed... http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator-backend-state.cc@294 PS8, Line 294: resource_utilization_.Merge(entry.second->resource_utilization_); I'm considering adjusting this to use an O(1) algorithm - Update() could return the delta in resource consumption and this could just be a Merge(). http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/runtime/coordinator.h@177 PS8, Line 177: backend This isn't accurate, could be fragment instances, backend or query. http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.h@135 PS8, Line 135:TQueryOptionLevel::ADVANCED)\ Need to fix tabs http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.cc File be/src/service/query-options.cc: http://gerrit.cloudera.org:8080/#/c/10415/8/be/src/service/query-options.cc@682 PS8, Line 682: // Parse the scan bytes limit and validate it. Useless comment. -- To view, visit http://gerrit.cloudera.org:8080/10415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20 Gerrit-Change-Number: 10415 Gerrit-PatchSet: 8 Gerrit-Owner: Mostafa Mokhtar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 19:12:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6034: Add CPU and scanned bytes limits per query
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10415 ) Change subject: IMPALA-6034: Add CPU and scanned bytes limits per query .. Patch Set 7: (5 comments) Addressed the previous comments and did various cleanup work. http://gerrit.cloudera.org:8080/#/c/10415/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10415/6//COMMIT_MSG@18 PS6, Line 18: Added tests for various permutations for CPU_LIMIT_S and > Something I noticed after playing around: How about we rename this to have Done http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc@1940 PS6, Line 1940: if (session_timeout <= 0) return; Fixed the null coordinator dereference and added a cancellation test that exercises the new code path. http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc@1970 PS6, Line 1970: is(); > This comment applies to my previous work here, but "expired" is actually ki Done http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc@1978 PS6, Line 1978: if (session_state.second->ref_count > 0) continue; > We still need to erase the event - now we have two copies of it so we've en Done http://gerrit.cloudera.org:8080/#/c/10415/6/be/src/service/impala-server.cc@1979 PS6, Line 1979: // A session closed by other means is in the process of being removed, and it's > nit: tabs Done -- To view, visit http://gerrit.cloudera.org:8080/10415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20 Gerrit-Change-Number: 10415 Gerrit-PatchSet: 7 Gerrit-Owner: Mostafa Mokhtar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 19:07:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6034: Add CPU and scanned bytes limits per query
Tim Armstrong has uploaded a new patch set (#7) to the change originally created by Mostafa Mokhtar. ( http://gerrit.cloudera.org:8080/10415 ) Change subject: IMPALA-6034: Add CPU and scanned bytes limits per query .. IMPALA-6034: Add CPU and scanned bytes limits per query To protect Impala from runaway queries add per query limits for CPU and scanned bytes, limits are enforced via query options and applied to the entire query, not per backend like mem_limit. If a query exceeds any of the limits it will get cancelled. Query profile is updated to include query wide and per backend metrics for Cpu and scanned bytes. Testing: Added tests for various permutations for CPU_LIMIT_S and SCAN_BYTES_LIMIT Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20 --- 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/service/impala-server.cc M be/src/service/impala-server.h M be/src/service/query-options-test.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/query-resource-limits.test M tests/query_test/test_cancellation.py M tests/query_test/test_resource_limits.py 14 files changed, 338 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/10415/7 -- To view, visit http://gerrit.cloudera.org:8080/10415 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I4c6015e21da684bb9f33e236d71309dd4c178a20 Gerrit-Change-Number: 10415 Gerrit-PatchSet: 7 Gerrit-Owner: Mostafa Mokhtar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Mostafa Mokhtar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/10857 ) Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Comment-Date: Tue, 03 Jul 2018 18:49:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6625: Skip computing parquet conjuncts for non-Parquet scans
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10704 ) Change subject: IMPALA-6625: Skip computing parquet conjuncts for non-Parquet scans .. Patch Set 14: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2772/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/10704 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d6c26d42db090c8a15c602f6419ad6399c329e7 Gerrit-Change-Number: 10704 Gerrit-PatchSet: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 18:45:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6625: Skip computing parquet conjuncts for non-Parquet scans
Pooja Nilangekar has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/10704 ) Change subject: IMPALA-6625: Skip computing parquet conjuncts for non-Parquet scans .. IMPALA-6625: Skip computing parquet conjuncts for non-Parquet scans This change ensures that the planner computes parquet conjuncts only when for scans containing parquet files. Additionally, it also handles PARQUET_DICTIONARY_FILTERING and PARQUET_READ_STATISTICS query options in the planner. Testing was carried out independently on parquet and non-parquet scans: 1. Parquet scans were tested via the existing parquet-filtering planner test. Additionally, a new test [parquet-filtering-disabled] was added to ensure that the explain plan generated skips parquet predicates based on the query options. 2. Non-parquet scans were tested manually to ensure that the functions to compute parquet conjucts were not invoked. Additional test cases were added to the parquet-filtering planner test to scan non parquet tables and ensure that the plans do not contain conjuncts based on parquet statistics. 3. A parquet partition was added to the alltypesmixedformat table in the functional database. Planner tests were added to ensure that Parquet conjuncts are constructed only when the Parquet partition is included in the query. Change-Id: I9d6c26d42db090c8a15c602f6419ad6399c329e7 --- M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java M testdata/bin/create-load-data.sh M testdata/bin/load-dependent-tables.sql M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test A testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test M testdata/workloads/functional-query/queries/QueryTest/mixed-format.test M testdata/workloads/functional-query/queries/QueryTest/show-stats.test M tests/query_test/test_rows_availability.py 14 files changed, 483 insertions(+), 34 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/10704/14 -- To view, visit http://gerrit.cloudera.org:8080/10704 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9d6c26d42db090c8a15c602f6419ad6399c329e7 Gerrit-Change-Number: 10704 Gerrit-PatchSet: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] [DOCS] Clarification on admission control and DDL statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10829 ) Change subject: [DOCS] Clarification on admission control and DDL statements .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Gerrit-Change-Number: 10829 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 18:41:46 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Clarification on admission control and DDL statements
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10829 ) Change subject: [DOCS] Clarification on admission control and DDL statements .. [DOCS] Clarification on admission control and DDL statements Removed the confusing example and paragraphs. Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Reviewed-on: http://gerrit.cloudera.org:8080/10829 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M docs/topics/impala_admission.xml 1 file changed, 61 insertions(+), 85 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Gerrit-Change-Number: 10829 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Tianyi Wang has restored this change. ( http://gerrit.cloudera.org:8080/10857 ) Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. Restored -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: restore Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Tianyi Wang has abandoned this change. ( http://gerrit.cloudera.org:8080/10857 ) Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-7236: Fix the parsing of ALLOW ERASURE CODED FILES
Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10857 Change subject: IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES .. IMPALA-7236: Fix the parsing of ALLOW_ERASURE_CODED_FILES This patch adds a missing "break" statement in a switch statement changed by IMPALA-7102. Also fixes an non-deterministic test case. Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 --- M be/src/service/query-options.cc M testdata/workloads/functional-query/queries/QueryTest/hdfs-erasure-coding.test 2 files changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/10857/1 -- To view, visit http://gerrit.cloudera.org:8080/10857 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ife1e791541e3f4fed6bec00945390c7d7681e824 Gerrit-Change-Number: 10857 Gerrit-PatchSet: 1 Gerrit-Owner: Tianyi Wang
[Impala-ASF-CR] IMPALA-7212: Deprecate --use krpc flag and remove old DataStream services
Sailesh Mukil has posted comments on this change. (
http://gerrit.cloudera.org:8080/10835 )
Change subject: IMPALA-7212: Deprecate --use_krpc flag and remove old
DataStream services
..
Patch Set 1:
(5 comments)
LGTM overall. Just some comments around the test code.
http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc
File be/src/runtime/data-stream-test.cc:
http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@23
PS1, Line 23: #include "common/status.h"
I think I've found all the dead code in this patch, but if you want a
reference, you can have a look at this patch to see if you need to remove
anything else:
https://github.com/apache/impala/commit/ff86feaa67ff8bf703896e33d9a358e42739ae30#diff-0b7021d2a8bfaf517b243f11a53bcde8
I added that when I was making this test KRPC compatible.
http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@128
PS1, Line 128: template class DataStreamTestBase : public T {
: protected:
: virtual void SetUp() {}
: virtual void TearDown() {}
: };
No longer needed.
http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@134
PS1, Line 134: enum KrpcSwitch {
: USE_THRIFT,
: USE_KRPC
: };
No longer needed.
http://gerrit.cloudera.org:8080/#/c/10835/1/be/src/runtime/data-stream-test.cc@139
PS1, Line 139: public DataStreamTestBase>
You can replace this with:
public testing::Test
http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py
File tests/custom_cluster/test_rpc_exception.py:
http://gerrit.cloudera.org:8080/#/c/10835/1/tests/custom_cluster/test_rpc_exception.py@a73
PS1, Line 73:
> We should probably look into removing certain injected exception if it's no
Can't this happen without TransmitData() today? Don't we also call it on
ReportExecStatus() and CancelQueryFInstances() ?
https://github.com/apache/impala/blob/fb8ea6a9ccaedc41a71f6f6dcb367fc1facd73b6/be/src/runtime/backend-client.h#L58-L76
--
To view, visit http://gerrit.cloudera.org:8080/10835
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icfed200751508478a3d728a917448f2dabfc67c3
Gerrit-Change-Number: 10835
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Tue, 03 Jul 2018 18:36:28 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [DOCS] Clarification on admission control and DDL statements
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10829 ) Change subject: [DOCS] Clarification on admission control and DDL statements .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/335/ -- To view, visit http://gerrit.cloudera.org:8080/10829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Gerrit-Change-Number: 10829 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 18:32:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7237: handle hex digits in ParseSmaps()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10853 ) Change subject: IMPALA-7237: handle hex digits in ParseSmaps() .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3dad846dafb25b414bee1858eb63f3eda31d59ac Gerrit-Change-Number: 10853 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Tue, 03 Jul 2018 18:29:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7237: handle hex digits in ParseSmaps()
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10853 ) Change subject: IMPALA-7237: handle hex digits in ParseSmaps() .. IMPALA-7237: handle hex digits in ParseSmaps() Testing: Manual. Added some temporary logging to print out which branch it took with each line and confirmed it took the right branch for a line starting with 'f'. Change-Id: I3dad846dafb25b414bee1858eb63f3eda31d59ac Reviewed-on: http://gerrit.cloudera.org:8080/10853 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/util/mem-info.cc 1 file changed, 2 insertions(+), 1 deletion(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/10853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3dad846dafb25b414bee1858eb63f3eda31d59ac Gerrit-Change-Number: 10853 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker
[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/10810 )
Change subject: IMPALA-7095: clean up scan node profiles
..
Patch Set 7:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/data-source-scan-node.cc
File be/src/exec/data-source-scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/data-source-scan-node.cc@359
PS7, Line 359: COUNTER_ADD(rows_read_counter_, rows_read);
Any idea why rows_returned_counter_ is updated at line 355 but
rows_read_counter_ is updated inside this if-stmt ? Should they be updated at
the same location for consistency ?
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/hbase-scan-node.h
File be/src/exec/hbase-scan-node.h:
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/hbase-scan-node.h@119
PS7, Line 119: time
wall clock time
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/hdfs-scan-node-base.h@126
PS7, Line 126: throughput
nit: adding (bytes/sec) like below in the definition of the field seems
helpful when reading this comment.
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/hdfs-scan-node-base.h@150
PS7, Line 150: WARN_UNUSED_RESULT;
nit: long line.
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/hdfs-scan-node-base.h@521
PS7, Line 521: //
nit: ///
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/kudu-scan-node-base.h
File be/src/exec/kudu-scan-node-base.h:
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/kudu-scan-node-base.h@41
PS7, Line 41:
: virtual Status Prepare(RuntimeState* state) override;
: virtual Status Open(RuntimeState* state) override;
: virtual Status GetNext(RuntimeState* state, RowBatch*
row_batch, bool* eos)
: override = 0;
: protected:
: virtual void DebugString(int indentation_level,
std::stringstream* out) const override;
WARN_UNUSED_RESULT;
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/kudu-scan-node.h
File be/src/exec/kudu-scan-node.h:
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/kudu-scan-node.h@44
PS7, Line 44: virtual Status Prepare(RuntimeState* state) override;
WARN_UNUSED_RESULT; Same below .
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h
File be/src/exec/scan-node.h:
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h@87
PS7, Line 87: the fragment execution thread
scanner threads
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h@202
PS7, Line 202: used by
only used by
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h@209
PS7, Line 209: class ScannerThreadState {
I suppose this class is mostly thread-safe as most fields are backed by
atomics, right ? The states can both be updated from both fragment execution
thread and scanner threads.
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h@211
PS7, Line 211: state
Mind elaborating ? This is mostly for initializing the counters, right ?
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h@214
PS7, Line 214: state
Open() creates the row batch queue, right ?
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h@255
PS7, Line 255:private:
nit: blank line before private makes it more legible.
http://gerrit.cloudera.org:8080/#/c/10810/7/be/src/exec/scan-node.h@271
PS7, Line 271: boost::scoped_ptr<
std::unique_ptr<> Aren't we moving away from boost in the long run ?
--
To view, visit http://gerrit.cloudera.org:8080/10810
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787
Gerrit-Change-Number: 10810
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Tue, 03 Jul 2018 18:00:43 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/10744/8/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/10744/8/common/thrift/ImpalaInternalService.thrift@931 PS8, Line 931: // Called to initiate shutdown of this backend. : TRemoteShutdownResult RemoteShutdown(1:TRemoteShutdownParams params); > Is it possible for me to port this to KRPC now? I guess when I wrote this p Yes, I think if you cherry-pick that patch mentioned above, you should be able to add a new RPC in control_service.proto and define the handler similar to what we did in ReportExecStatus() RPC. Again, didn't mean to hold you back so if this patch goes in first, we can do the porting as a separate patch. Looking at this patch, this should hopefully be straightforward albeit may be some question about timeout which I may need to take a closer look. -- To view, visit http://gerrit.cloudera.org:8080/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 17:23:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/10744/8/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/10744/8/common/thrift/ImpalaInternalService.thrift@931 PS8, Line 931: // Called to initiate shutdown of this backend. : TRemoteShutdownResult RemoteShutdown(1:TRemoteShutdownParams params); > It's unfortunate timing but we are moving away from Thrift to KRPC (https:/ Is it possible for me to port this to KRPC now? I guess when I wrote this patch we could run without KRPC but that's being removed now. -- To view, visit http://gerrit.cloudera.org:8080/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 17:19:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7190: Remove unsupported format writer support
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10823 ) Change subject: IMPALA-7190: Remove unsupported format writer support .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I821dc7495a901f1658daa500daf3791b386c7185 Gerrit-Change-Number: 10823 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 17:16:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7190: Remove unsupported format writer support
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10823 ) Change subject: IMPALA-7190: Remove unsupported format writer support .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2771/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I821dc7495a901f1658daa500daf3791b386c7185 Gerrit-Change-Number: 10823 Gerrit-PatchSet: 4 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 17:16:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1760: Implement shutdown command
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10744 ) Change subject: IMPALA-1760: Implement shutdown command .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/10744/8/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/10744/8/common/thrift/ImpalaInternalService.thrift@931 PS8, Line 931: // Called to initiate shutdown of this backend. : TRemoteShutdownResult RemoteShutdown(1:TRemoteShutdownParams params); It's unfortunate timing but we are moving away from Thrift to KRPC (https://gerrit.cloudera.org/#/c/10855/). I suppose we just need to port this over to KRPC later once this patch is merged unless the aforementioned patch happens to go in first. -- To view, visit http://gerrit.cloudera.org:8080/10744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4d5606ccfec84db4482c1e7f0f198103aad141a0 Gerrit-Change-Number: 10744 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 17:06:33 + Gerrit-HasComments: Yes
[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 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.cc@437
PS2, Line 437:
: }
: VLOG_QUERY << "descriptor table for query=" <<
PrintId(query_id())
: << "\n" << desc_tbl_->DebugString();
:
: Status thread_create_status;
: DCHECK_GT(rpc_params_.fragment_ctxs.size(), 0);
: TPlanFragmentCtx* fragment_ctx = _params_.fragment_ctxs[0];
: int fragment_ctx_idx = 0;
: fragment_events_start_time_ = MonotonicStopWatch::Now();
: for (const TPlanFragmentInstanceCtx& instance_ctx:
rpc_params_.fragment_instance_ctxs) {
: // determine corresponding TPlanFragmentCtx
: if (fragment_ctx->fragment.idx != instance_ctx.fragment_idx)
{
: ++fragment_ctx_idx;
:
> My point was shouldn't the counting barrier take care of that for us? I.e.
CountingBarrier::Notify() does not set the promise unless it's the one to
reduce the count to 0 (or in other words only the last fragment instance to
complete preparing would get to set the promise).
So if a first fragment instance hits an error in Prepare() while the others are
still running Prepare(), then it won't be able to set the error unless it has
some way of communicating its error to the other fragment instances, or it
waits until the rest of them are done and then sets the error.
So, we'd need some other mechanism of allowing us to both:
1) Save the first error, and
2) Wait for all the FInstances to complete Prepare()
Unless I'm missing something?
http://gerrit.cloudera.org:8080/#/c/10813/2/be/src/runtime/query-state.cc@586
PS2, Line 586:
:
:
:
:
:
> Right, the references are held by FIS for resources shared across the query
As we spoke in person, I will roll this part out as a separate patch to make
reviewing this a bit easier.
--
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: 4
Gerrit-Owner: Sailesh Mukil
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Comment-Date: Tue, 03 Jul 2018 17:00:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5981: [DOCS] Documented SET=""
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10816 ) Change subject: IMPALA-5981: [DOCS] Documented SET="" .. IMPALA-5981: [DOCS] Documented SET="" Also, refactored the Impala SET doc and moved the command SET to the Impala Shell Commands doc. Change-Id: I7211405d5cc0a548c05ea5218798591873c14417 Reviewed-on: http://gerrit.cloudera.org:8080/10816 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M docs/topics/impala_set.xml M docs/topics/impala_shell_commands.xml 2 files changed, 105 insertions(+), 225 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10816 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7211405d5cc0a548c05ea5218798591873c14417 Gerrit-Change-Number: 10816 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-5981: [DOCS] Documented SET=""
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10816 ) Change subject: IMPALA-5981: [DOCS] Documented SET="" .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10816 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7211405d5cc0a548c05ea5218798591873c14417 Gerrit-Change-Number: 10816 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 16:52:41 + Gerrit-HasComments: No
[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 4:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@65
PS3, Line 65: state denot
> I think we should avoid using the term "query state" given that this class
Done
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@65
PS3, Line 65: dExecState. We
> Backend
Done
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@68
PS3, Line 68: to the ERROR or CANCELLED
> are there multiple error states? corresponding implies that. Or are you inc
I reworded it to be clearer now. It can only either be ERROR or CANCELLED.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@72
PS3, Line 72: This is to simplify the
: /// query lifecycle so that Prepare() is always completed befor
> Let's not state in terms of past code. How about:
Yes, technically Cancel() or PublishFilter(). So I've re-worded it to that.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@149
PS3, Line 149: s belonging to this query. Each instance receives its own
execution
> put another way, I guess this blocks until threads have exited INITIALIZED?
Yes, I've added that wording to make it clearer.
The reason I kept MonitorFInstances() separately is because the status
reporting will be driven from there after IMPALA-2990, making it more readable
than keeping it in StartFInstances().
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@155
PS3, Line 155: /// Monitors all the fragment instances and updates the
BackendExecState according
> it it required that this is called after StartFInstances()?
Yes, I've added that to the comments now.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@210
PS3, Line 210: return strings::Substitute("Status: $0 (fra
> Is that needed in C++? You'd do it for C but I think C++ effectively inclu
You're right. Removed it.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@213
PS3, Line 213:
> nit: here and below you could delete the word "Function" since that's obvio
Done
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@266
PS3, Line 266: dExecS
> Isn't Cancel() called in case of error as well? If so, does this mean we go
Yes, that's true. I've reworded it to specify only CancelQueryFInstances() RPC
and a coordinator directed CANCEL from a response to ReportExecStatus() RPC.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@271
PS3, Line 271: repare(). Implies
> how about backend_exec_state_ for consistency?
Done
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@284
PS3, Line 284: u
> here and elsewhere: given BackendExecState is a scalar (enum) value, should
Done
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@286
PS3, Line 286: or E
> from
Done
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@305
PS3, Line 305: ar* Ba
> overall status
Done
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.h@306
PS3, Line 306:
> otherwise it's the status of the first non-OK FIS?
Yes, added that to the comment.
Also, I changed this to a FInstanceStatus. I wanted to do that initially, but I
forgot to.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.cc
File be/src/runtime/query-state.cc:
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.cc@201
PS3, Line 201: const char* QueryState::BackendExecStateToString(const
BackendExecState& state) {
: static const unordered_map
exec_state_to_str{
: {BackendExecState::INITIALIZED, "INITIALIZED"},
: {BackendExecState::PREPARED, "PREPARED"},
{BackendExecState::FINISHED, "FINISHED"},
: {BackendExecState::CANCELLED, "CANCELLED"},
{BackendExecState::ERROR, "ERROR"}};
: retu
> given that the function is inlined this is probably unnecessary. dead code
Good point. Done.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.cc@226
PS3, Line 226: << BackendExecStateToString(old_state) << ")";
> why doesn't that include FINISHED? This check doesn't seem very intuitive t
After removing the 2 states I had before, this seems pretty unnecessary now. I
removed it.
http://gerrit.cloudera.org:8080/#/c/10813/3/be/src/runtime/query-state.cc@266
PS3, Line 266: ested cancellati
> how about UpdateBackendExecState (since QueryState is the name of the class
Done
[Impala-ASF-CR] IMPALA-7163: Implement a state machine for the QueryState class
Hello Michael Ho, Joe McDonnell, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10813 to look at the new patch set (#4). 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 7 possible states for the QueryState which include 3 terminal states (FINISHED, CANCELLED and ERROR) and 4 non-terminal states (INITIALIZED, PREPARED). 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-2990, which is the primary reason behind having only this thread modify the state of the query. 2 counting barriers are introduced which are used to indicate how many fragment instances have finished Preparing and Executing. We use CountingBarriers to allow fragment instance threads to communicate their completion of their respective states or errors with the QueryState thread. Due to this design, it's possible for the fragment instances to have collectively moved on to future states while the query state thread lags behind in realizing that. However, that's not a concern for us since we only care about showing a unified view of what's happening on this executor to the coordinator (post IMPALA-2990). The status reporting protocol has not been changed and remains exactly as it was. Testing: Ran 'core' and 'exhaustive' tests. TODO: Run stress 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/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/query-state.h 5 files changed, 328 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/13/10813/4 -- 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: 4 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-5981: [DOCS] Documented SET=""
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10816 ) Change subject: IMPALA-5981: [DOCS] Documented SET="" .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-docs-submit/334/ -- To view, visit http://gerrit.cloudera.org:8080/10816 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7211405d5cc0a548c05ea5218798591873c14417 Gerrit-Change-Number: 10816 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 16:43:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7224. Improve performance of UpdateCatalogMetrics
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10846 ) Change subject: IMPALA-7224. Improve performance of UpdateCatalogMetrics .. IMPALA-7224. Improve performance of UpdateCatalogMetrics This function is called after every DDL query, and was implemented by fetching the entire list of table names, even though only the length of that list was needed. In workloads with millions of tables, this could add several seconds of overhead following even simple requests like 'USE' or 'DESCRIBE'. I tested a backported version of this patch against one such workload. It reduced the time taken for a simple DESCRIBE query from 12-14sec down to about 40ms. I also tested locally that the metrics on impalad were still updated by DDL operations. Change-Id: Ic5467adbce1e760ff93996925db5611748efafc0 Reviewed-on: http://gerrit.cloudera.org:8080/10846 Reviewed-by: Vuk Ercegovac Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins --- M be/src/service/frontend.cc M be/src/service/frontend.h M be/src/service/impala-server.cc M common/thrift/Frontend.thrift M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java 6 files changed, 40 insertions(+), 11 deletions(-) Approvals: Vuk Ercegovac: Looks good to me, approved Tim Armstrong: Looks good to me, but someone else must approve Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10846 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic5467adbce1e760ff93996925db5611748efafc0 Gerrit-Change-Number: 10846 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tianyi Wang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-7237: handle hex digits in ParseSmaps()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10853 ) Change subject: IMPALA-7237: handle hex digits in ParseSmaps() .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/10853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3dad846dafb25b414bee1858eb63f3eda31d59ac Gerrit-Change-Number: 10853 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Tue, 03 Jul 2018 15:05:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7237: handle hex digits in ParseSmaps()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10853 ) Change subject: IMPALA-7237: handle hex digits in ParseSmaps() .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2770/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/10853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3dad846dafb25b414bee1858eb63f3eda31d59ac Gerrit-Change-Number: 10853 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Tue, 03 Jul 2018 15:05:33 + Gerrit-HasComments: No
[Impala-ASF-CR] [DOCS] Clarification on admission control and DDL statements
Balazs Jeszenszky has posted comments on this change. ( http://gerrit.cloudera.org:8080/10829 ) Change subject: [DOCS] Clarification on admission control and DDL statements .. Patch Set 3: Thanks Tim for clarifying the session-thing. I was thinking in impala-shell context only. -- To view, visit http://gerrit.cloudera.org:8080/10829 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2e3e82bd34e88e7a13de1864aeb97f01023bc715 Gerrit-Change-Number: 10829 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 10:55:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7095: clean up scan node profiles
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10810 ) Change subject: IMPALA-7095: clean up scan node profiles .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/10810 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77286282d42e7764bfdf94c7ec47cec9d743f787 Gerrit-Change-Number: 10810 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 03 Jul 2018 09:37:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4784: Remove InProcessStatestore
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10843 ) Change subject: IMPALA-4784: Remove InProcessStatestore .. IMPALA-4784: Remove InProcessStatestore InProcessStatestore was only used by statestore-test, expr-test and session-expiry-test. With a slight refactor of the Statestore class, InProcessStatestore becomes obsolete. This patch moves the ownership of the ThriftServer into the Statestore class and Statestore::Init() now takes a 'port' parameter instead of using the FLAGS_state_store_port directly. We also remove the InProcessStatestore completely. A follow on patch will get rid of the InProcessImpalaServer too (IMPALA-6013) Testing: Ran 'core' tests. Change-Id: I2621873e593b36c9612a6402ac6c5d8e3b49cde9 Reviewed-on: http://gerrit.cloudera.org:8080/10843 Reviewed-by: Sailesh Mukil Tested-by: Impala Public Jenkins --- M be/src/exprs/expr-test.cc M be/src/service/session-expiry-test.cc M be/src/statestore/statestore-test.cc M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M be/src/statestore/statestored-main.cc M be/src/testutil/in-process-servers.cc M be/src/testutil/in-process-servers.h 8 files changed, 95 insertions(+), 125 deletions(-) Approvals: Sailesh Mukil: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/10843 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I2621873e593b36c9612a6402ac6c5d8e3b49cde9 Gerrit-Change-Number: 10843 Gerrit-PatchSet: 7 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4784: Remove InProcessStatestore
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10843 ) Change subject: IMPALA-4784: Remove InProcessStatestore .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/10843 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2621873e593b36c9612a6402ac6c5d8e3b49cde9 Gerrit-Change-Number: 10843 Gerrit-PatchSet: 6 Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 03 Jul 2018 08:21:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3040: Remove cache directive before dropping a table
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/10792 )
Change subject: IMPALA-3040: Remove cache directive before dropping a table
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10792/2/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/10792/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1317
PS2, Line 1317: if (db != null) {
I don't think this is the right approach. Firstly this is still prone to races.
Secondly, if the dropDatabase() in L1324 fails for some reason, we'd have
unnecessarily uncached the tables eagerly.
How about fixing the HdfsPartition.dropPartition() to also cleanup the
partition directive? (refer to my example in the CR comment).
--
To view, visit http://gerrit.cloudera.org:8080/10792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7701a499405e961456adea63f3592b43bd69170
Gerrit-Change-Number: 10792
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Tianyi Wang
Gerrit-Comment-Date: Tue, 03 Jul 2018 07:47:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3040: Remove cache directive before dropping a table
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/10792 )
Change subject: IMPALA-3040: Remove cache directive before dropping a table
..
Patch Set 2:
Thanks for the explanation. I guess I understand the issue now.
The basic problem here seems to be that HdfsTable.dropPartition() does not
clean up cache directives of the partitions. We do it inside
CatalogOpExecutor#alterTableDropPartition()
private Table alterTableDropPartition(Table tbl,.)
..
if (part.isMarkedCached()) {
HdfsCachingUtil.removePartitionCacheDirective(part);
}
}
but since these partitions are dropped already using Hive, Impala clears the
state with dropPartition() and uncacheTable() later does not help anymore.
Here is a simple repro of this issue.
// Create a partitioned cached table and add some partitions
impala> create table cached_tbl_part (i int) partitioned by (j int) cached in
'testPool'
impala> insert into cached_tbl_part (i,j) select 1, 2;
// Make sure cache directives are created.
$ hdfs cacheadmin -listDirectives | grep cached_tbl_part
277 testPool1 never /test-warehouse/cached_tbl_part
278 testPool1 never /test-warehouse/cached_tbl_part/j=2 <
// Drop the partition from hive
hive> alter table cached_tbl_part drop partition (j=2);
// Refresh the table from Impala
impala> refresh cached_tbl_part;
// Cache directives still exist
hdfs cacheadmin -listDirectives | grep cached_tbl_part
277 testPool1 never /test-warehouse/cached_tbl_part
278 testPool1 never /test-warehouse/cached_tbl_part/j=2 <--- should
have been dropped
// Drop the table from Impala
impala> drop table cached_tbl_part;
$ hdfs cacheadmin -listDirectives | grep cached_tbl_part
278 testPool1 never /test-warehouse/cached_tbl_part/j=2 <---
Table's directive is dropped but the partition still remains.
--
To view, visit http://gerrit.cloudera.org:8080/10792
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id7701a499405e961456adea63f3592b43bd69170
Gerrit-Change-Number: 10792
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Tianyi Wang
Gerrit-Comment-Date: Tue, 03 Jul 2018 07:43:37 +
Gerrit-HasComments: No
