[Impala-ASF-CR] IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14714 ) Change subject: IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3 .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.cc File be/src/runtime/datetime-parser-common.cc: http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.cc@235 PS5, Line 235: *token_end -= buff.length() - prefix.length() - expected_suffix.length(); Maybe this would be more straightforward: *token_end = token_start + prefix.length() + expected_suffix.length(); -- To view, visit http://gerrit.cloudera.org:8080/14714 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f Gerrit-Change-Number: 14714 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 28 Nov 2019 07:09:03 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 28 Nov 2019 03:44:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. IMPALA-9126: part 3: move more logic to PhjBuilder The general flavour of this patch is to move code that orchestrates the top-level spilling hash join algorithm to PhjBuilder, and better encapsulate state in PhjBuilder by reducing the number of public methods. Specific changes include: * Move HashJoinState to PhjBuilder, which is necessary for the shared join build since the builder will be orchestrating the spilling. * Reduce public methods of PhjBuilder. The goal is for the builder to hand off pointers to hash tables, partitions, etc only during transitions of the state machine (i.e. synchronization points when we have a shared build). * Highlight methods of PhjBuilder that will be synchronization points for the shared join build where hash tables are built and destroyed. * Highlight other future changes to PhjBuilder. * Move some of the output build partition logic from NextSpilledProbeRowBatch() to the builder, which required a few other changes - e.g. explicit *eos output arguments for some functions. This does *not* include a change to have the builder pick the spilled partition to process. That will be a follow-on, because it requires more refactoring of the relationship between PhjBuilder::Partition and ProbePartition. Testing: * Earlier version of patch passed exhaustive tests. Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Reviewed-on: http://gerrit.cloudera.org:8080/14787 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node-ir.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h 5 files changed, 554 insertions(+), 392 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7550: Add documentation to profile counters
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14776 ) Change subject: IMPALA-7550: Add documentation to profile counters .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5168/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7 Gerrit-Change-Number: 14776 Gerrit-PatchSet: 2 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 28 Nov 2019 02:13:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7550: Add documentation to profile counters
Jiawei Wang has posted comments on this change. (
http://gerrit.cloudera.org:8080/14776 )
Change subject: IMPALA-7550: Add documentation to profile counters
..
Patch Set 2:
(9 comments)
Hi Tim,
Thanks for the advice. The second round combined stability with significance.
Now counters have 4 types which are STABLE_HIGH, STABLE_LOW, UNSTABLE, DEBUG
Also, the second round I replaced all counters in scan-node.cc
One thing I am not sure is that whether we should remove the previous comments
on all the counters?
Please let me know if you have other thoughts.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.h@525
PS1, Line 525: /// Disk accessed bitmap
> These are weird cause they're not actually included in the profile directly
Done
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/hdfs-scan-node-base.cc@127
PS1, Line 127:
PROFILE_DEFINE_SUMMARY_STATS_COUNTER(ParquetUncompressedBytesReadPerColumn,
STABLE_LOW,
> I'm OK with the redundant text, but it might be reasonable to use a templat
hmm, I will just leave it here then because we only have two counters and the
contents are subject to change.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/scan-node.cc
File be/src/exec/scan-node.cc:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/scan-node.cc@82
PS1, Line 82: PROFILE_DEFINE_TIMER(RowBatchQueueGetWaitTime, UNSTABLE, "Wall
clock time that the "
> We could probably define an enum or something like that. I'd be ok with jus
I will just leave it in the description.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/exec/scan-node.cc@95
PS1, Line 95:
PROFILE_DEFINE_HIGH_WATER_MARK_COUNTER(PeakScannerThreadConcurrency, DEBUG,
> I think doing this makes sense - that would force all counters to be define
Done
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/runtime/coordinator-backend-state.cc@60
PS1, Line 60:
> This approach of converting the namespaces where needed seems fine to me. T
Okay, will adopt the second approach whenever needed.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h@154
PS1, Line 154: // Debugging counters - generally not useful to users of
Impala, the main use case is
> nit: formatting is weird, I'd expect the enum values to only be indented tw
Done
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h@164
PS1, Line 164: const char* name() const { return name_; }
> I think we should probably combine them, because they're pretty correlated
Okay, I will split stable into high_stable and low_stable and keep unstable and
debug the same.
http://gerrit.cloudera.org:8080/#/c/14776/1/be/src/util/runtime-profile-counters.h@189
PS1, Line 189: /// prototypes will register with the singleton instance of this
class. Then, this class
> Are you planning to do this?
I dont think so, unless you feel like we will need this. Otherwise I will just
delete this
http://gerrit.cloudera.org:8080/#/c/14776/1/www/profile_docs.tmpl
File www/profile_docs.tmpl:
http://gerrit.cloudera.org:8080/#/c/14776/1/www/profile_docs.tmpl@1
PS1, Line 1:
[Impala-ASF-CR] IMPALA-7550: Add documentation to profile counters
Jiawei Wang has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/14776 ) Change subject: IMPALA-7550: Add documentation to profile counters .. IMPALA-7550: Add documentation to profile counters This is the work based on Lars' experiment on https://gerrit.cloudera.org/#/c/12116/ This commit did the following refactors on profile counters. 1. Add a singleton registry for runtime profile counters prototypes, similiar to what Kudu does for metrics. This allows us to generate profile documentation for all counters from the code. We add /profile_docs and a correspoding UI for the documentation of profile counters. 2. Profile counters are annotated with their stability: * Stable_HIGH counters - High level and stable counters. Always useful on measuring query performance and status. Counters that everyone is interested. Should rarely change and if it does we will make some effort to notify users. * Stable_LOW - Low level and sable counters - interesting counters to monitor and analyze by machine. It will probably be interesting under some circumstance for users. Lots of developers are interested. * Unstable but useful - useful to understand query performance, but subject to change, particularly if the implementation changes. E.g. RowBatchQueuePutWaitTime, MaterializeTupleTimer * Debugging counters - generally not useful to users of Impala, the main use case is low-level debugging. Can be hidden to reduce noise for most consumers of profiles. 3. We have around 250 counters. This commit did the replacement in scan-node and hdfs-scan-node-base and coordinator. This is still a WIP work and all the advices are welcomed! The downside is that we'd reduce the comments that currently explain some of the counters in header files by moving them to the .cc files. Additionally a (arguably good) limitation is that profile counter names need to be unique. Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7 --- M be/src/exec/hbase-scan-node.cc M be/src/exec/hbase-scan-node.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-scan-node-base.h M be/src/exec/kudu-scan-node-base.cc M be/src/exec/scan-node.cc M be/src/exec/scan-node.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator.cc D be/src/util/debug-counters.h M be/src/util/default-path-handlers.cc M be/src/util/runtime-profile-counters.h M be/src/util/runtime-profile.cc A www/profile_docs.tmpl M www/query_profile.tmpl 15 files changed, 620 insertions(+), 292 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/76/14776/2 -- To view, visit http://gerrit.cloudera.org:8080/14776 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idc03faddb27754001290bb6d899840e2cbe7ccb7 Gerrit-Change-Number: 14776 Gerrit-PatchSet: 2 Gerrit-Owner: Jiawei Wang Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Balazs Jeszenszky Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jiawei Wang Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 ) Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5303/ -- To view, visit http://gerrit.cloudera.org:8080/14744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630 Gerrit-Change-Number: 14744 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Thu, 28 Nov 2019 00:54:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4080: [Part 1] Move static state from ExecNode into a PlanNode
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14764 )
Change subject: IMPALA-4080: [Part 1] Move static state from ExecNode into a
PlanNode
..
Patch Set 5:
(20 comments)
The big picture looks good. Obviously there's a bunch more refactoring steps so
trying to focus on the bits that matter at this point.
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/aggregation-node-base.h
File be/src/exec/aggregation-node-base.h:
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/aggregation-node-base.h@36
PS5, Line 36: /// Performs the actual work of aggregating input rows.
Comment is a little misleading. Maybe "Configuration for each aggregate class".
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/aggregation-node-base.cc
File be/src/exec/aggregation-node-base.cc:
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/aggregation-node-base.cc@65
PS5, Line 65: // TODO: maybe use a similar createNode method like in PlanNode.
Remove comment? The suggested refactoring seems fine, but also isn't too
necessary.
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/aggregation-node-base.cc@68
PS5, Line 68: num_ags
nit: num_aggs
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h
File be/src/exec/exec-node.h:
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@57
PS5, Line 57: /// TODO: Move all the static state of ExecNode into PlanNode.
Mention a JIRA here?
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@66
PS5, Line 66: /// do any initialization that can fail in Init() rather than
the ctor.
There isn't a constructor, so comment needs fixing. I was actually wondering
though why you didn't do most of the initialisation in the constructors, since
most of it can't fail. Was that just a desire to avoid having
I guess compared to ExecNode there shouldn't be mutable state in the plan node.
Would it be worth documenting that state is expected to be constant after
Init()? Maybe this is all simple enough that that's not a big deal.
I think you can ignore this if you want, just trying to think things through.
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@68
PS5, Line 68: call
nit: called
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@71
PS5, Line 71: pool
Do you mean state->obj_pool() or similar?
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@83
PS5, Line 83: const TPlanNode* tnode_ = nullptr;
It's a little weird that all of the members with underscores are public.
I think this kinda makes sense as an intermediate step in the refactoring,
because adding accessor methods is a pain, but would be good to leave a comment
in PlanNode indicating the direction this is going.
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@90
PS5, Line 90: RowDescriptor* row_descriptor_ = nullptr;
What is this owned by? Same for other objects. Or maybe we should document that
all objects should be owned by a particular ObjectPool?
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@102
PS5, Line 102: private:
Should add a DISALLOW_COPY_AND_ASSIGN(PlanNode) here to suppress copy
constructors, etc. It would be good to do this in general for classes that
shouldn't be copied, but maybe just add it here and ExecNode?
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@194
PS5, Line 194: TODO: revisit this line.
?
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@195
PS5, Line 195: lanNode*
should maybe be a const ref?
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@358
PS5, Line 358: const T& GetPlanNode() { return dynamic_cast(plan_node_); }
DCHECK that the cast pointer isn't null? I.e. that the dynamic cast succeeded.
Another thought: we might want to do static_cast for perf reasons if this
function is used in any loops (because dynamic_cast needs to do a runtime
check). The DCHECK would still be useful in debug modes though.
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.h@382
PS5, Line 382: RowDescriptor& row_descriptor_;
const ref? Unless it results in a cascade of changes.
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.cc
File be/src/exec/exec-node.cc:
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.cc@140
PS5, Line 140: stringstream error_msg;
Not a huge deal and you're copying the existing pattern, but I think we should
declare this only in the default: branch where it's used. That should work so
long as the block is wrapped in braces.
http://gerrit.cloudera.org:8080/#/c/14764/5/be/src/exec/exec-node.cc@317
PS5, Line 317: plan_root
root is a little misleading since this isn't going to be the root of the whole
tree on a recursive call. Maybe just 'plan_node'? Ok to ignore.
http://gerrit.clo
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5167/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 23:49:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14755 ) Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5166/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca Gerrit-Change-Number: 14755 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 27 Nov 2019 23:40:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14756 ) Change subject: IMPALA-9151: Maintain cluster size in ExecutorMembershipSnapshot .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14756 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6b05326c82fb3ca625c015cfcdc38f891f5d4f9 Gerrit-Change-Number: 14756 Gerrit-PatchSet: 3 Gerrit-Owner: Lars Volker Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 23:25:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 23:04:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5304/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 23:04:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 23:04:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9202: Fix flakiness in test executor groups
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14810 ) Change subject: IMPALA-9202: Fix flakiness in test_executor_groups .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5165/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14810 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47070045250a12d86c99f9a30a956a268be5fa7e Gerrit-Change-Number: 14810 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 27 Nov 2019 23:04:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/14787/6/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: http://gerrit.cloudera.org:8080/#/c/14787/6/be/src/exec/partitioned-hash-join-builder.h@214 PS6, Line 214: /// they are in-memory with a hash table built or have build_rows() prepared for reading. > line too long (91 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 23:04:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Hello Michael Ho, Thomas Tauber-Marshall, Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14787 to look at the new patch set (#7). Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. IMPALA-9126: part 3: move more logic to PhjBuilder The general flavour of this patch is to move code that orchestrates the top-level spilling hash join algorithm to PhjBuilder, and better encapsulate state in PhjBuilder by reducing the number of public methods. Specific changes include: * Move HashJoinState to PhjBuilder, which is necessary for the shared join build since the builder will be orchestrating the spilling. * Reduce public methods of PhjBuilder. The goal is for the builder to hand off pointers to hash tables, partitions, etc only during transitions of the state machine (i.e. synchronization points when we have a shared build). * Highlight methods of PhjBuilder that will be synchronization points for the shared join build where hash tables are built and destroyed. * Highlight other future changes to PhjBuilder. * Move some of the output build partition logic from NextSpilledProbeRowBatch() to the builder, which required a few other changes - e.g. explicit *eos output arguments for some functions. This does *not* include a change to have the builder pick the spilled partition to process. That will be a follow-on, because it requires more refactoring of the relationship between PhjBuilder::Partition and ProbePartition. Testing: * Earlier version of patch passed exhaustive tests. Change-Id: I0e233468de1eeae86651ab96df207de19e091053 --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node-ir.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h 5 files changed, 554 insertions(+), 392 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/14787/7 -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14755 ) Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring .. Patch Set 5: I decided to remove the changes to the ImpalaHttpHandler. The changes don't make much sense without the necessary transparent query retry code + I found a few issues in the approach while working on my query retry POC. -- To view, visit http://gerrit.cloudera.org:8080/14755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca Gerrit-Change-Number: 14755 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 27 Nov 2019 22:56:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9124: ImpalaServer and ClientRequestState refactoring
Hello Michael Ho, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14755 to look at the new patch set (#5). Change subject: IMPALA-9124: ImpalaServer and ClientRequestState refactoring .. IMPALA-9124: ImpalaServer and ClientRequestState refactoring Re-factoring several areas of the ImpalaServer and ClientRequestState in preparation for future work required for transparent query retries. Re-factored TExecRequest ownership in ClientRequestState. Previously, the TExecRequest was passed to the ClientRequestState via the Exec(TExecRequest) method. Now it is created and owned by the ClientRequestState and only exposed as a constant reference. It is stored as a plain member variable. Re-factored ImpalaServer::UnregisterQuery into multiple methods. Made ClientRequestStateMap a dedicated class that extends ShardedQueryMap with additional methods to add and delete a ClientRequestState. The re-factoring breaks up the large UnregisterQuery method into multiple smaller method and adds some additional code documentation. Re-factored ImpalaServer::client_request_state_map_ into a separate class. Testing: * Ran core tests Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca --- A be/src/service/client-request-state-map.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-server.cc M be/src/service/impala-server.h 5 files changed, 143 insertions(+), 57 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/14755/5 -- To view, visit http://gerrit.cloudera.org:8080/14755 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib92c932a69802225af3fd9bf15f85c85317acaca Gerrit-Change-Number: 14755 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9202: Fix flakiness in test executor groups
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14810 ) Change subject: IMPALA-9202: Fix flakiness in test_executor_groups .. Patch Set 1: Code-Review+1 LGTM -- To view, visit http://gerrit.cloudera.org:8080/14810 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47070045250a12d86c99f9a30a956a268be5fa7e Gerrit-Change-Number: 14810 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 27 Nov 2019 22:51:52 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5164/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 22:35:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 22:29:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14641 ) Change subject: IMPALA-8138: Reintroduce rpc debugging options .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Gerrit-Change-Number: 14641 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 22:27:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14641 ) Change subject: IMPALA-8138: Reintroduce rpc debugging options .. IMPALA-8138: Reintroduce rpc debugging options In the past, Impala had a very simple 'fault injection' framework for simulating failed rpcs between impalads. With the move to KRPC, that framework was not carried over, and we lost the ability to test certain failure scenarios. This patch reintroduces this functionality. It removes the prior fault injection framework in favor of the existing debug action framework, which is more flexible. To facilitate this, a few modifications are made to the debug action framework: - In addition to matching on a label, debug actions may now match on optional arguments. In this patch, the debug action IMPALA_SERVICE_POOL takes the arguments 'host', 'port', and 'rpc name' to allow simulating the failure of specific rpcs to specific impalads. - The FAIL action now takes an optional 'error message' parameter. In this patch, the debug action IMPALA_SERVICE_POOL uses this to simulate different types of rpc errors, eg. 'service too busy'. - The FAIL action increments a metric, 'impala.debug_action.fail', so that tests can check that it has actually been hit. Prior to this patch the tests in test_rpc_exception.py where all passing spuriously as the faults they were supposed to be testing were no longer being injected. This patch uses these new mechanisms to introduce tests that simulate failures in DataStreamService rpcs. Follow up patches will add test cases for ControlService rpcs. Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Reviewed-on: http://gerrit.cloudera.org:8080/14641 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/rpc/impala-service-pool.cc M be/src/rpc/impala-service-pool.h M be/src/rpc/rpc-mgr-kerberized-test.cc M be/src/rpc/rpc-mgr-test.cc M be/src/rpc/rpc-mgr-test.h M be/src/rpc/rpc-mgr.cc M be/src/rpc/rpc-mgr.h M be/src/runtime/backend-client.h M be/src/runtime/data-stream-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/test-env.cc M be/src/service/control-service.cc M be/src/service/data-stream-service.cc M be/src/service/impala-internal-service.cc M be/src/testutil/CMakeLists.txt D be/src/testutil/fault-injection-util.cc D be/src/testutil/fault-injection-util.h M be/src/util/debug-util.cc M be/src/util/debug-util.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M common/thrift/ImpalaService.thrift M common/thrift/metrics.json M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/custom_cluster/test_rpc_exception.py 26 files changed, 242 insertions(+), 281 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Gerrit-Change-Number: 14641 Gerrit-PatchSet: 10 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9202: Fix flakiness in test executor groups
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14810 Change subject: IMPALA-9202: Fix flakiness in test_executor_groups .. IMPALA-9202: Fix flakiness in test_executor_groups Some tests in test_executor_groups immediately tried fetching the query profile after executing it asynchronously to verify if the query was queued. However there is a small window between the exec rpc returning and the query being queued during which the query profile does not contain any info about the query being queued. This was causing some asserts in the test to fail. Change-Id: I47070045250a12d86c99f9a30a956a268be5fa7e --- M tests/custom_cluster/test_executor_groups.py 1 file changed, 15 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/14810/1 -- To view, visit http://gerrit.cloudera.org:8080/14810 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I47070045250a12d86c99f9a30a956a268be5fa7e Gerrit-Change-Number: 14810 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/14787/6/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: http://gerrit.cloudera.org:8080/#/c/14787/6/be/src/exec/partitioned-hash-join-builder.h@214 PS6, Line 214: /// they are in-memory with a hash table built or have build_rows() prepared for reading. line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 21:52:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Hello Michael Ho, Thomas Tauber-Marshall, Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14787 to look at the new patch set (#6). Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. IMPALA-9126: part 3: move more logic to PhjBuilder The general flavour of this patch is to move code that orchestrates the top-level spilling hash join algorithm to PhjBuilder, and better encapsulate state in PhjBuilder by reducing the number of public methods. Specific changes include: * Move HashJoinState to PhjBuilder, which is necessary for the shared join build since the builder will be orchestrating the spilling. * Reduce public methods of PhjBuilder. The goal is for the builder to hand off pointers to hash tables, partitions, etc only during transitions of the state machine (i.e. synchronization points when we have a shared build). * Highlight methods of PhjBuilder that will be synchronization points for the shared join build where hash tables are built and destroyed. * Highlight other future changes to PhjBuilder. * Move some of the output build partition logic from NextSpilledProbeRowBatch() to the builder, which required a few other changes - e.g. explicit *eos output arguments for some functions. This does *not* include a change to have the builder pick the spilled partition to process. That will be a follow-on, because it requires more refactoring of the relationship between PhjBuilder::Partition and ProbePartition. Testing: * Earlier version of patch passed exhaustive tests. Change-Id: I0e233468de1eeae86651ab96df207de19e091053 --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node-ir.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h 5 files changed, 553 insertions(+), 392 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/14787/6 -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@183 PS5, Line 183: ) > typo: I think you intended the ')' to go after true Done http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@185 PS5, Line 185: instead prepares build rows for reading > This confused me a bit, as it makes it sound like we don't prepare build ro I thought about this and realised that this isn't really part of the visible behaviour of BeginSpilledProbe(), it's just a precursor to returning the partition in DoneProbingSinglePartition(). So I removed this paragraph and augmented the other comment. http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@502 PS5, Line 502: HashJoinState state_ = HashJoinState::PARTITIONING_BUILD; > Might want to put this in the section below noting things that need to be R Done http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@524 PS5, Line 524: /// Set in FlushFinal() and constant thereafter. > Also gets Reset() Good point. Done http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.cc@68 PS5, Line 68: partitions_created_(nullptr), > Maybe do all of these in the header instead Done http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.cc@496 PS5, Line 496: state_ == HashJoinState::PARTITIONING_PROBE ? 0 : 1 > Brief comment what the extra buffer is for Done -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 21:51:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 ) Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. Patch Set 3: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/543/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 27 Nov 2019 20:51:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9122 : Ignore FileNotFoundException when loading a table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14806 ) Change subject: IMPALA-9122 : Ignore FileNotFoundException when loading a table .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5163/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14806 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iecf6b193b0d57de27d41ad6ef6e1719005d9e908 Gerrit-Change-Number: 14806 Gerrit-PatchSet: 1 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Wed, 27 Nov 2019 20:49:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14677 ) Change subject: IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5162/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c Gerrit-Change-Number: 14677 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 20:46:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 ) Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/543/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 27 Nov 2019 20:27:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Hello Gabor Kaszab, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14722 to look at the new patch set (#3). Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 - The following format specifiers are documented: - FX - FM - Free text Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 --- M docs/topics/impala_conversion_functions.xml 1 file changed, 376 insertions(+), 396 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/14722/3 -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 ) Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml File docs/topics/impala_conversion_functions.xml: http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@549 PS2, Line 549: The following rules apply to string: > This doesn't make sense for me. Done http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@560 PS2, Line 560: to have the same : length > "to have the maximum possible length." Done http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@589 PS2, Line 589: Valid only for the date/time to string conversions. > The FM modifier can be used in either datetime to string and string to date Done http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@633 PS2, Line 633: double quotes in the free text : token > double quotes surrounding the free text token Done http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@800 PS2, Line 800: > nit: please remove the trailing space here and one line below. Done -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 27 Nov 2019 20:27:42 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Reviewed-on: http://gerrit.cloudera.org:8080/14802 Reviewed-by: Tim Armstrong Tested-by: Impala Public Jenkins Reviewed-by: Thomas Tauber-Marshall --- M docs/topics/impala_ssl.xml 1 file changed, 67 insertions(+), 55 deletions(-) Approvals: Tim Armstrong: Looks good to me, but someone else must approve Impala Public Jenkins: Verified Thomas Tauber-Marshall: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 ) Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630 Gerrit-Change-Number: 14744 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 20:13:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6894: Use an internal representation of query states in ClientRequestState
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14744 ) Change subject: IMPALA-6894: Use an internal representation of query states in ClientRequestState .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5303/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14744 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ce70bd2e964b309ebfc9d6ff6d900485db4d630 Gerrit-Change-Number: 14744 Gerrit-PatchSet: 4 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 20:13:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9122 : Ignore FileNotFoundException when loading a table
Vihang Karajgaonkar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14806 Change subject: IMPALA-9122 : Ignore FileNotFoundException when loading a table .. IMPALA-9122 : Ignore FileNotFoundException when loading a table It is possible that when the file metadata of a table or partition is being loaded, some temporary files (like the ones in .hive-staging directory) are deleted by external engines like Hive. This causes a FileNotFoundException during the load and it fails the reload command. In general, this should not be a problem since users are careful not to modify the table from Hive or Spark while Impala is reading them. In the worst case, currently the refresh command fails which can be retried by the user. However, the this does not go well with when event processing is turned on. EventProcessor tries to reload the table as soon as it sees a INSERT_EVENT from metastore. Hive may be still cleaning up the staging directories when EventProcessor issues a reload causing it go in error state. Ideally, we should have some sort of intra-engine synchronization semantics to avoid such issues, but that is much more complex architectural change. For now, we should ignore such errors and skip the deleted file from being loaded. Testing: [Pending] Unfortunately, this error is hard to reproduce locally. I tried creating multiple threads which delete some files while multiple FileMetadataLoaders are loading concurrently but it didn't fail for me so far. Will try running the TestEventProcessing.test_insert_events multiple times to confirm if I see any failure. Change-Id: Iecf6b193b0d57de27d41ad6ef6e1719005d9e908 --- M fe/src/main/java/org/apache/impala/common/FileSystemUtil.java 1 file changed, 16 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14806/1 -- To view, visit http://gerrit.cloudera.org:8080/14806 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iecf6b193b0d57de27d41ad6ef6e1719005d9e908 Gerrit-Change-Number: 14806 Gerrit-PatchSet: 1 Gerrit-Owner: Vihang Karajgaonkar Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14677 ) Change subject: IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails .. Patch Set 3: Basically re-wrote the patch. * The RPCErrorMessage information is now encapsulated in the StatusAuxInfoPB struct * The StatusAuxInfoPB is tracked in the RuntimeState of each fragment instance * The StatusAuxInfoPB is propagated to the Coordinator via ReportExecStatusRequestPB * Removed all the Status re-factoring / changes -- To view, visit http://gerrit.cloudera.org:8080/14677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c Gerrit-Change-Number: 14677 Gerrit-PatchSet: 3 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 20:04:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails
Hello Michael Ho, Thomas Tauber-Marshall, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14677 to look at the new patch set (#6). Change subject: IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails .. IMPALA-9137: Blacklist node if a DataStreamService RPC to the node fails Introduces a new optional field to ReportExecStatusRequestPB: StatusAuxInfoPB. StatusAuxInfoPB contains optional metadata associated with ReportExecStatusRequestPB::overall_status. Currently, StatusAuxInfoPB only contains one field: RPCErrorMessagePB, which is only set if the ReportExecStatusRequestPB::fragment_instance_id failed because a RPC to another impalad failed. The RPCErrorMessagePB contains the destination node of the failed RPC. Coordinator::UpdateBackendExecStatus(ReportExecStatusRequestPB, ...) uses the information in RPCErrorMessagePB (if one is set) to blacklist the target node. Testing: * Ran core tests * Planning to add more tests after IMPALA-8138 is merged Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/krpc-data-stream-sender.cc M be/src/runtime/query-state.cc M be/src/runtime/runtime-state.h A be/src/runtime/status-aux-info.h M be/src/util/network-util.cc M be/src/util/network-util.h M common/protobuf/common.proto M common/protobuf/control_service.proto 10 files changed, 168 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/77/14677/6 -- To view, visit http://gerrit.cloudera.org:8080/14677 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I733cca13847fde43c8ea2ae574d3ae04bd06419c Gerrit-Change-Number: 14677 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 19:43:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. Patch Set 3: Verified+1 Build Successful https://jenkins.impala.io/job/gerrit-docs-auto-test/542/ : Doc tests passed. -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 19:40:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9182: Print the socket address of the client closing a session or cancelling a query from the WebUI
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14782 ) Change subject: IMPALA-9182: Print the socket address of the client closing a session or cancelling a query from the WebUI .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/14782/7/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/14782/7/tests/webserver/test_web_pages.py@743 PS7, Line 743: self.assert_impalad_log_contains > This is all a little brittle cause when you run a test cluster with start-i Oh yeah, the dockerised case is another good point. Most of the assert_impalad_log_contains uses are for custom cluster tests, which start a new cluster just for that test. -- To view, visit http://gerrit.cloudera.org:8080/14782 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf74ad06ce1c40fab4ce37de6b7ca78e3e520b43 Gerrit-Change-Number: 14782 Gerrit-PatchSet: 7 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Wed, 27 Nov 2019 19:36:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h File be/src/exec/partitioned-hash-join-builder.h: http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@183 PS5, Line 183: ) typo: I think you intended the ')' to go after true http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@185 PS5, Line 185: instead prepares build rows for reading This confused me a bit, as it makes it sound like we don't prepare build rows for reading in other (non-empty_probe) cases, which I don't think is correct. http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@502 PS5, Line 502: HashJoinState state_ = HashJoinState::PARTITIONING_BUILD; Might want to put this in the section below noting things that need to be Reset() http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.h@524 PS5, Line 524: /// Set in FlushFinal() and constant thereafter. Also gets Reset() http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.cc File be/src/exec/partitioned-hash-join-builder.cc: http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.cc@68 PS5, Line 68: partitions_created_(nullptr), Maybe do all of these in the header instead http://gerrit.cloudera.org:8080/#/c/14787/5/be/src/exec/partitioned-hash-join-builder.cc@496 PS5, Line 496: state_ == HashJoinState::PARTITIONING_PROBE ? 0 : 1 Brief comment what the extra buffer is for -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 19:36:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9182: Print the socket address of the client closing a session or cancelling a query from the WebUI
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14782 )
Change subject: IMPALA-9182: Print the socket address of the client closing a
session or cancelling a query from the WebUI
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14782/7/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:
http://gerrit.cloudera.org:8080/#/c/14782/7/tests/webserver/test_web_pages.py@743
PS7, Line 743: self.assert_impalad_log_contains
> I think I need to reset the log directory location here. assert_log_contain
This is all a little brittle cause when you run a test cluster with
start-impala-cluster.py for development, it puts the logs in /cluster, but when
it's run as part of a full test run, it puts the logs in /ee_tests.
It's more important for this to pass as part of the full test rul, so
assert_log_contains() is doing the right thing by default. It's just annoying
for local development. If you start your test cluster with the below command it
should be equivalent to the jenkins run...
start-impala-cluster.py" --log_dir="${IMPALA_EE_TEST_LOGS_DIR}"
--
To view, visit http://gerrit.cloudera.org:8080/14782
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf74ad06ce1c40fab4ce37de6b7ca78e3e520b43
Gerrit-Change-Number: 14782
Gerrit-PatchSet: 7
Gerrit-Owner: Vincent Tran
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: Vincent Tran
Gerrit-Comment-Date: Wed, 27 Nov 2019 19:34:26 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9182: Print the socket address of the client closing a session or cancelling a query from the WebUI
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/14782 ) Change subject: IMPALA-9182: Print the socket address of the client closing a session or cancelling a query from the WebUI .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/14782/7/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/14782/7/tests/webserver/test_web_pages.py@743 PS7, Line 743: self.assert_impalad_log_contains > I think I need to reset the log directory location here. assert_log_contain I dug a bit more. It looks like assert_impalad_log_contains() fails because gerrit-verify-dryrun runs as a Dockerized cluster. TestLogFragments::test_log_fragments() also uses assert_impalad_log_contains() and it needs the @SkipIfDockerizedCluster.daemon_logs_not_exposed decorator. -- To view, visit http://gerrit.cloudera.org:8080/14782 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf74ad06ce1c40fab4ce37de6b7ca78e3e520b43 Gerrit-Change-Number: 14782 Gerrit-PatchSet: 7 Gerrit-Owner: Vincent Tran Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vincent Tran Gerrit-Comment-Date: Wed, 27 Nov 2019 19:30:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 19:20:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/14802/2/docs/topics/impala_ssl.xml File docs/topics/impala_ssl.xml: http://gerrit.cloudera.org:8080/#/c/14802/2/docs/topics/impala_ssl.xml@125 PS2, Line 125: (--ssl_minimum_version=tlsv1.2), the Python version > Unfortunately the story is a bit more complicated, cause Red Hat backported Done http://gerrit.cloudera.org:8080/#/c/14802/2/docs/topics/impala_ssl.xml@126 PS2, Line 126: on the cluster > that impala-shell is run with Done -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 19:17:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Hello Thomas Tauber-Marshall, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14802 to look at the new patch set (#3). Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ Change-Id: I681bb7d9900f00402259758d579f396da9cd007d --- M docs/topics/impala_ssl.xml 1 file changed, 67 insertions(+), 55 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/14802/3 -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. Patch Set 3: Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/542/ Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstr...@cloudera.com or on this JIRA: IMPALA-7317 -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 19:17:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14641 ) Change subject: IMPALA-8138: Reintroduce rpc debugging options .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5161/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Gerrit-Change-Number: 14641 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 18:31:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/14802/2/docs/topics/impala_ssl.xml File docs/topics/impala_ssl.xml: http://gerrit.cloudera.org:8080/#/c/14802/2/docs/topics/impala_ssl.xml@125 PS2, Line 125: (--ssl_minimum_version=tlsv1.2), the Python version Unfortunately the story is a bit more complicated, cause Red Hat backported some of these fixes to Python 2.7.5 - see IMPALA-8595. Enough users of Impala would be affected by this that it's probably worth mentioning, e.g. "... must be 2.7.9 or higher (or a vendor-provided Python version with the required support, e.g. some patched Python 2.7.5 versions on Red Hat Enterprise Linux 7 and derivatives)." -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 18:19:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14802 ) Change subject: IMPALA-9198: [DOCS] impala-shell works with tlsv1.2 only with python 2.7.9+ .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/14802/2/docs/topics/impala_ssl.xml File docs/topics/impala_ssl.xml: http://gerrit.cloudera.org:8080/#/c/14802/2/docs/topics/impala_ssl.xml@126 PS2, Line 126: on the cluster that impala-shell is run with -- To view, visit http://gerrit.cloudera.org:8080/14802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I681bb7d9900f00402259758d579f396da9cd007d Gerrit-Change-Number: 14802 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 17:59:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14641 ) Change subject: IMPALA-8138: Reintroduce rpc debugging options .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Gerrit-Change-Number: 14641 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 17:47:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14641 ) Change subject: IMPALA-8138: Reintroduce rpc debugging options .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5302/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Gerrit-Change-Number: 14641 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 17:47:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14641 ) Change subject: IMPALA-8138: Reintroduce rpc debugging options .. Patch Set 8: Code-Review+2 (1 comment) carrying forward http://gerrit.cloudera.org:8080/#/c/14641/7/be/src/util/debug-util.h File be/src/util/debug-util.h: http://gerrit.cloudera.org:8080/#/c/14641/7/be/src/util/debug-util.h@146 PS7, Line 146: const std::vector > const-reference Done -- To view, visit http://gerrit.cloudera.org:8080/14641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Gerrit-Change-Number: 14641 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Wed, 27 Nov 2019 17:47:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8138: Reintroduce rpc debugging options
Hello Michael Ho, Sahil Takiar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14641 to look at the new patch set (#8). Change subject: IMPALA-8138: Reintroduce rpc debugging options .. IMPALA-8138: Reintroduce rpc debugging options In the past, Impala had a very simple 'fault injection' framework for simulating failed rpcs between impalads. With the move to KRPC, that framework was not carried over, and we lost the ability to test certain failure scenarios. This patch reintroduces this functionality. It removes the prior fault injection framework in favor of the existing debug action framework, which is more flexible. To facilitate this, a few modifications are made to the debug action framework: - In addition to matching on a label, debug actions may now match on optional arguments. In this patch, the debug action IMPALA_SERVICE_POOL takes the arguments 'host', 'port', and 'rpc name' to allow simulating the failure of specific rpcs to specific impalads. - The FAIL action now takes an optional 'error message' parameter. In this patch, the debug action IMPALA_SERVICE_POOL uses this to simulate different types of rpc errors, eg. 'service too busy'. - The FAIL action increments a metric, 'impala.debug_action.fail', so that tests can check that it has actually been hit. Prior to this patch the tests in test_rpc_exception.py where all passing spuriously as the faults they were supposed to be testing were no longer being injected. This patch uses these new mechanisms to introduce tests that simulate failures in DataStreamService rpcs. Follow up patches will add test cases for ControlService rpcs. Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 --- M be/src/rpc/impala-service-pool.cc M be/src/rpc/impala-service-pool.h M be/src/rpc/rpc-mgr-kerberized-test.cc M be/src/rpc/rpc-mgr-test.cc M be/src/rpc/rpc-mgr-test.h M be/src/rpc/rpc-mgr.cc M be/src/rpc/rpc-mgr.h M be/src/runtime/backend-client.h M be/src/runtime/data-stream-test.cc M be/src/runtime/exec-env.cc M be/src/runtime/test-env.cc M be/src/service/control-service.cc M be/src/service/data-stream-service.cc M be/src/service/impala-internal-service.cc M be/src/testutil/CMakeLists.txt D be/src/testutil/fault-injection-util.cc D be/src/testutil/fault-injection-util.h M be/src/util/debug-util.cc M be/src/util/debug-util.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h M common/thrift/ImpalaService.thrift M common/thrift/metrics.json M tests/common/impala_cluster.py M tests/common/impala_service.py M tests/custom_cluster/test_rpc_exception.py 26 files changed, 242 insertions(+), 281 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/41/14641/8 -- To view, visit http://gerrit.cloudera.org:8080/14641 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9c047ebce6d32c5ae461f70279391fa2df4c2029 Gerrit-Change-Number: 14641 Gerrit-PatchSet: 8 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9174: Speedup allocations of ORC Scanner
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14804 ) Change subject: IMPALA-9174: Speedup allocations of ORC Scanner .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5160/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia09e746883176d6f955c1718267bf55e2abb239b Gerrit-Change-Number: 14804 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 17:46:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9174: Speedup allocations of ORC Scanner
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14804 ) Change subject: IMPALA-9174: Speedup allocations of ORC Scanner .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc@74 PS1, Line 74: return addr + sizeof(SizeType); This could cause problems if the caller is assuming that this is more than 8-byte aligned. I think malloc() gives 16 byte alignment by default. I think it's unlikely that ORC is assuming 16 byte alignment but might be worth confirming (maybe they're using int128_t for decimals or similar - we had seen issues with some SIMD instructions on 16-byte decimals faulting within Impala because they did aligned loads - we had to force unaligned loads with memcpy() in those cases). -- To view, visit http://gerrit.cloudera.org:8080/14804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia09e746883176d6f955c1718267bf55e2abb239b Gerrit-Change-Number: 14804 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 17:25:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9174: Speedup allocations of ORC Scanner
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14804 ) Change subject: IMPALA-9174: Speedup allocations of ORC Scanner .. Patch Set 1: (1 comment) The solution makes sense to me. I originally was thinking that you could have used the malloc extensions (we have that dependency anyway), but I think that gets complicated because they don't return the exact size that you allocated. http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/14804/1/be/src/exec/hdfs-orc-scanner.cc@67 PS1, Line 67: char* addr = static_cast(std::malloc(sizeof(SizeType) + size)); TCMalloc has an API to get the size of an allocation: https://github.com/gperftools/gperftools/blob/f47a52ce85c3d8d559aaae7b7a426c359fbca225/src/gperftools/tcmalloc.h.in#L118 There's also an equivalent for the sanitizer malloc - https://github.com/llvm-mirror/compiler-rt/blob/master/include/sanitizer/allocator_interface.h#L31 We already use both these different extensions, e.g. in be/src/util/memory-metrics.cc, so it's not a new dependency. I guess the sizes are a little different because they include fragmentation overhead. Oh right, and we need to consume the memory before allocating anyway, so we don't know the allocated size when we call Consume(). So this idea probably doesn't work out. -- To view, visit http://gerrit.cloudera.org:8080/14804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia09e746883176d6f955c1718267bf55e2abb239b Gerrit-Change-Number: 14804 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 17:20:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9174: Speedup allocations of ORC Scanner
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14804 Change subject: IMPALA-9174: Speedup allocations of ORC Scanner .. IMPALA-9174: Speedup allocations of ORC Scanner The ORC library provides a hook for its clients to plugin a custom memory pool. This memory pool needs to override the 'malloc()' and 'free()' methods. Impala has its own memory pool named OrcMemPool. Impala's OrcMemPool used to have an internal unordered_map to keep track of its allocations. In 'free()' it used the map to lookup the size of the allocated byte array. We need this information in 'free()' because of memory tracking. Therefore for each 'malloc()' and 'free()' there was an additional allocation/deallocation by the unordered_map. Instead of using a map this patch allocates a few more bytes on each allocation and store the size of the allocated bytes in front of the allocated bytes, similarly to how the standard malloc/free works. (Unfortunately the standard malloc/free doesn't reveal that information in a standard way). OrcMemPool also had a method called 'FreeAll()' which freed all allocated memory. This was a no-op because the library only uses the memory pool to allocate memory for the data buffers, and they free their memory in their destructors. On the other hand, it provided some kind of guard against memory leaks in the ORC library. We can add some checks to the destructor of OrcMemPool to detect leaks if we don't trust the library's memory management. Change-Id: Ia09e746883176d6f955c1718267bf55e2abb239b --- M be/src/exec/hdfs-orc-scanner.cc M be/src/exec/hdfs-orc-scanner.h 2 files changed, 7 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/14804/1 -- To view, visit http://gerrit.cloudera.org:8080/14804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia09e746883176d6f955c1718267bf55e2abb239b Gerrit-Change-Number: 14804 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 ) Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5159/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7 Gerrit-Change-Number: 14665 Gerrit-PatchSet: 4 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Anonymous Coward (536) Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Comment-Date: Wed, 27 Nov 2019 16:25:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/14714 )
Change subject: IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
..
Patch Set 5:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-iso-sql-format-parser.cc
File be/src/runtime/datetime-iso-sql-format-parser.cc:
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-iso-sql-format-parser.cc@297
PS5, Line 297: if (tok.type == MONTH_NAME && fx_provided && !tok.fm_modifier) {
: if (input_len < MAX_MONTH_NAME_LENGTH) return nullptr;
: return input_str + MAX_MONTH_NAME_LENGTH;
: }
What happens if tok.type == MONTH_NAME && (!fx_provided || tok.fm_modifier)?
I think in that case, FindEndOfToken() doesn't return the proper end of the
token, since month names are not necessarily followed by a separator.
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h
File be/src/runtime/datetime-parser-common.h:
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@43
PS5, Line 43: const int MONTH_LENGTHS[12] = {31, 28, 31, 30, 31, 30, 31, 31,
30, 31, 30, 31};
Is this still used?
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@57
PS5, Line 57: number
nit: ordinal number
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@63
PS5, Line 63: number
nit: ordinal number
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@70
PS5, Line 70: /// Mapping between textual month name and the number of month.
Used for string to
: /// datetime conversions.
: const std::unordered_maphttp://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@88
PS5, Line 88: 'FEB
closing ' is missing.
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@295
PS5, Line 295: /// If the month part of the input is not followed by a
separator then the end of the
: /// month part is found by probing the input with different
lengths.
Please fix the comment.
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@308
PS5, Line 308: MONTH_LENGTHS
MONTH_RANGES and LEAP_YEAR_MONTH_RANGES
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.h@328
PS5, Line 328: one
nit: 1
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.cc
File be/src/runtime/datetime-parser-common.cc:
http://gerrit.cloudera.org:8080/#/c/14714/5/be/src/runtime/datetime-parser-common.cc@193
PS5, Line 193: const char* input_end
This parametr is not really needed. It is used only in a DCHECK.
--
To view, visit http://gerrit.cloudera.org:8080/14714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f
Gerrit-Change-Number: 14714
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 27 Nov 2019 16:11:48 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Gabor Kaszab has posted comments on this change. (
http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@197
PS3, Line 197: public String getCastFormatWithEscapedSingleQuotes() {
: Preconditions.checkNotNull(castFormat_);
: Preconditions.checkState(!castFormat_.isEmpty());
: StringBuilder result = new StringBuilder();
:
> This doesn't work under every circumstance, e.g.:
That library seems to be deprecated for me (moved to different location). I
didn't wabt toi include a dependency on that so I wrote some parsing to escape
the single quotes and take into account the preceeding escaped backslashes.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Norbert Luksa
Gerrit-Comment-Date: Wed, 27 Nov 2019 15:44:26 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Hello Norbert Luksa, Anonymous Coward (536), Attila Jeges, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14665 to look at the new patch set (#4). Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST .. IMPALA-9131: Use single quotes around FORMAT clause in CAST When running a CAST(..FORMAT..) query then the header of the output shows the value of the FORMAT clause surrounded by double quotes. However, the SQL way is to use single quotes for strings so this patch changes the printout from using double quotes to use single quotes instead. Additionally, this fixes a bug where it wasn't possible to have a single quote separator in a format string that was itself surrounded by single quotes. As a fix the single quote separator can be escaped by a backslash in this case. Another additional fix to make the FX modifier case-insensitive. Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7 --- M be/src/runtime/datetime-iso-sql-format-parser.cc M be/src/runtime/datetime-iso-sql-format-tokenizer.cc M be/src/runtime/datetime-iso-sql-format-tokenizer.h M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M tests/query_test/test_cast_with_format.py 6 files changed, 118 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/4 -- To view, visit http://gerrit.cloudera.org:8080/14665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7 Gerrit-Change-Number: 14665 Gerrit-PatchSet: 4 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Anonymous Coward (536) Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa
[Impala-ASF-CR] IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14714 ) Change subject: IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3 .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5158/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14714 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f Gerrit-Change-Number: 14714 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 27 Nov 2019 12:45:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
..
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@197
PS3, Line 197: public String getCastFormatWithEscapedSingleQuotes() {
: Preconditions.checkNotNull(castFormat_);
: Preconditions.checkState(!castFormat_.isEmpty());
: return castFormat_.replaceAll("([^])'", "$1'");
: }
This doesn't work under every circumstance, e.g.:
- unescaped apostrophe at the beginning of the format string:
select cast("'2010-02-01" as date format "'-mm-dd");
the printout in the header is incorrect:
cast('\'2010-02-01' as date format ''-mm-dd')
- escaped \ in front of an unescaped apostrophe in a text token:
select cast("2010-\\'-02-01" as date FORMAT "FX-\"\\'\"-MM-DD");
the printout in the header is incorrect:
cast('2010-\\\'-02-01' as date format 'fx-\"\\'\"-mm-dd')
Maybe you could use something from the StringEscapeUtils class in
org.apache.commons.lang3 package?
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Norbert Luksa
Gerrit-Comment-Date: Wed, 27 Nov 2019 12:19:12 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
Gabor Kaszab has posted comments on this change. (
http://gerrit.cloudera.org:8080/14714 )
Change subject: IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
..
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc
File be/src/runtime/datetime-parser-common.cc:
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc@212
PS4, Line 212: if (fx_modifier && !tok.fm_modifier) {
: DCHECK(buff.length() == MAX_MONTH_NAME_LENGTH);
: trim_right_if(buff, is_any_of(" "));
: }
> This block is probably not necessary. The parsing below should succeed with
If I remove this block then L233-234 would fail for "padded" month names (like
"May ").
Additionaly L233-234 is needed not to accept "2010-JANUARY-10" with
"FX-MONTH-DD" where the month name is not padded to 9 length.
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc@225
PS4, Line 225: const char* actual_suffix = buff.c_str() +
SHORT_MONTH_NAME_LENGTH;
: if (strncmp(actual_suffix, expected_suffix.c_str(),
expected_suffix.length()) != 0) {
: return false;
: }
> It would be faster to use a const char* pointer and strncmp() for comparis
Done
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc@232
PS4, Line 232: // separator then the end of the month token has
> No need to create a new string instance. Only the length of 'month_name' is
Done
--
To view, visit http://gerrit.cloudera.org:8080/14714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f
Gerrit-Change-Number: 14714
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 27 Nov 2019 12:02:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
Hello Attila Jeges, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14714 to look at the new patch set (#5). Change subject: IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3 .. IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3 This patch adds additional datetime format tokens on top of Milestone 1 (IMPALA-8703) and Milestone 2 (IMPALA-8704). The tokens introduced: - Full month name (MONTH, Month, month): In a string to datetime conversion this token can parse textual month name into a datetime type. In a datetime to string conversion this token gives the textual representation of a month. - Short month name (MON, Mon, mon): Similar to the full month name token but this works for 3-character month names like 'JAN'. - Full day name (DAY, Day, day): In a datetime to string conversion this token gives the textual representation of a day like 'Tuesday.' Not suppported in a string to datetime conversion. - Short day name (DY, Dy, dy): Similar to full day name token but this works for 3-character day names like 'TUE'. - Day of week (D): In a datetime to string conversion this gives a number in [1-7] where 1 represents Sunday. Not supported in a string to datetime conversion. - Quarter of year (Q): In a datetime to string conversion this gives a number in [1-4] representing a quarter of the year. Not supported in a string to datetime conversion. - Week of year (WW): In a datetime to string conversion this gives a number in [1-53] to represent the week of year where the first week starts from 1st of January. Not supported in a string to datetime conversion. - Week of month (W): In a datetime to string conversion this gives a number in [1-5] to represent the week of month where the first week starts from the first day of the month. Not supported in a string to datetime conversion. Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f --- M be/src/benchmarks/convert-timestamp-benchmark.cc M be/src/benchmarks/parse-timestamp-benchmark.cc M be/src/common/init.cc M be/src/exprs/date-functions-ir.cc M be/src/exprs/timestamp-functions-ir.cc M be/src/exprs/timestamp-functions.cc M be/src/exprs/timestamp-functions.h M be/src/runtime/date-parse-util.cc M be/src/runtime/date-parse-util.h M be/src/runtime/datetime-iso-sql-format-parser.cc M be/src/runtime/datetime-iso-sql-format-parser.h M be/src/runtime/datetime-iso-sql-format-tokenizer.cc M be/src/runtime/datetime-parser-common.cc M be/src/runtime/datetime-parser-common.h M be/src/runtime/datetime-simple-date-format-parser.cc M be/src/runtime/datetime-simple-date-format-parser.h M be/src/runtime/timestamp-parse-util.cc M tests/query_test/test_cast_with_format.py 18 files changed, 1,063 insertions(+), 101 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/14/14714/5 -- To view, visit http://gerrit.cloudera.org:8080/14714 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f Gerrit-Change-Number: 14714 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 ) Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5157/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7 Gerrit-Change-Number: 14665 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Anonymous Coward (536) Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Comment-Date: Wed, 27 Nov 2019 10:30:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
Attila Jeges has posted comments on this change. (
http://gerrit.cloudera.org:8080/14714 )
Change subject: IMPALA-8705: ISO:SQL:2016 datetime patterns - Milestone 3
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc
File be/src/runtime/datetime-parser-common.cc:
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc@212
PS4, Line 212: if (fx_modifier && !tok.fm_modifier) {
: DCHECK(buff.length() == MAX_MONTH_NAME_LENGTH);
: trim_right_if(buff, is_any_of(" "));
: }
This block is probably not necessary. The parsing below should succeed without
trimming 'buff'.
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc@225
PS4, Line 225: const string& actual_suffix =
: buff.substr(SHORT_MONTH_NAME_LENGTH,
expected_suffix.length());
: if (actual_suffix != expected_suffix) return false;
: *month = it->second.second;
It would be faster to use a const char* pointer and strncmp() for comparison
instead of creating an 'actual_suffix' string instance.
http://gerrit.cloudera.org:8080/#/c/14714/4/be/src/runtime/datetime-parser-common.cc@232
PS4, Line 232: const string month_name = prefix + actual_suffix;
No need to create a new string instance. Only the length of 'month_name' is
used in L233-236.
--
To view, visit http://gerrit.cloudera.org:8080/14714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic797f19a1311b54e5d00d01d0a7afe1f0f21fb8f
Gerrit-Change-Number: 14714
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 27 Nov 2019 10:25:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 ) Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2 .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml File docs/topics/impala_conversion_functions.xml: http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@549 PS2, Line 549: The following rules apply to string: This doesn't make sense for me. "The following rules apply:" should be enough. http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@560 PS2, Line 560: to have the same : length "to have the maximum possible length." http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@589 PS2, Line 589: Valid only for the date/time to string conversions. The FM modifier can be used in either datetime to string and string to datetime scenarios so this is not correct. What I meant with the comment is that the description you had for "FM" is true only for the datetime to string path. Please remove this line and rephrase the above like: "In a datetime to string conversion FM suppresses blank paddig..." "In a string to datetime conversion FM is used for omitting the effect of FX for certain tokens." (or something similar :) ) http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@633 PS2, Line 633: double quotes in the free text : token double quotes surrounding the free text token http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@800 PS2, Line 800: nit: please remove the trailing space here and one line below. -- To view, visit http://gerrit.cloudera.org:8080/14722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07 Gerrit-Change-Number: 14722 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 27 Nov 2019 10:23:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Gabor Kaszab has posted comments on this change. (
http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@237
PS2, Line 237: ++(*current_pos);
> Giving this a second thought it sounds a bit weird if an IsSomething() func
Well, leaving it this way wouldn't cause any harm as calling this function
multiple times would still produce the same result. Anyway, I haven't found any
better solution.
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@246
PS2, Line 246: strncasecmp(*current_pos, "FX"
> Not this change, but i think this should be strncasecmp(..) instead. Format
I added the fix to this patch (didn't want to create a new jira and handle it
separately.)
http://gerrit.cloudera.org:8080/#/c/14665/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@209
PS2, Line 209: } else {
> This still needs to be fixed:
Indeed, i have fixed everything but the original issue with my previous patches
:)
Done
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Norbert Luksa
Gerrit-Comment-Date: Wed, 27 Nov 2019 09:48:24 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Hello Norbert Luksa, Anonymous Coward (536), Attila Jeges, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14665 to look at the new patch set (#3). Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST .. IMPALA-9131: Use single quotes around FORMAT clause in CAST When running a CAST(..FORMAT..) query then the header of the output shows the value of the FORMAT clause surrounded by double quotes. However, the SQL way is to use single quotes for strings so this patch changes the printout from using double quotes to use single quotes instead. Additionally, this fixes a bug where it wasn't possible to have a single quote separator in a format string that was itself surrounded by single quotes. As a fix the single quote separator can be escaped by a backslash in this case. Another additional fix to make the FX modifier case-insensitive. Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7 --- M be/src/runtime/datetime-iso-sql-format-parser.cc M be/src/runtime/datetime-iso-sql-format-tokenizer.cc M be/src/runtime/datetime-iso-sql-format-tokenizer.h M fe/src/main/java/org/apache/impala/analysis/CastExpr.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java M tests/query_test/test_cast_with_format.py 6 files changed, 87 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/3 -- To view, visit http://gerrit.cloudera.org:8080/14665 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7 Gerrit-Change-Number: 14665 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Anonymous Coward (536) Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/5156/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 08:54:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 5: Rebase only -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 08:11:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14787 ) Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/14787/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14787/5//COMMIT_MSG@41 PS5, Line 41: * Rerun exhaustive tests with latest changes. Need to remove this. -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Wed, 27 Nov 2019 08:11:46 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9126: part 3: move more logic to PhjBuilder
Hello Michael Ho, Thomas Tauber-Marshall, Csaba Ringhofer, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14787 to look at the new patch set (#5). Change subject: IMPALA-9126: part 3: move more logic to PhjBuilder .. IMPALA-9126: part 3: move more logic to PhjBuilder The general flavour of this patch is to move code that orchestrates the top-level spilling hash join algorithm to PhjBuilder, and better encapsulate state in PhjBuilder by reducing the number of public methods. Specific changes include: * Move HashJoinState to PhjBuilder, which is necessary for the shared join build since the builder will be orchestrating the spilling. * Reduce public methods of PhjBuilder. The goal is for the builder to hand off pointers to hash tables, partitions, etc only during transitions of the state machine (i.e. synchronization points when we have a shared build). * Highlight methods of PhjBuilder that will be synchronization points for the shared join build where hash tables are built and destroyed. * Highlight other future changes to PhjBuilder. * Move some of the output build partition logic from NextSpilledProbeRowBatch() to the builder, which required a few other changes - e.g. explicit *eos output arguments for some functions. This does *not* include a change to have the builder pick the spilled partition to process. That will be a follow-on, because it requires more refactoring of the relationship between PhjBuilder::Partition and ProbePartition. Testing: * Earlier version of patch passed exhaustive tests. TODO: * Rerun exhaustive tests with latest changes. Change-Id: I0e233468de1eeae86651ab96df207de19e091053 --- M be/src/exec/partitioned-hash-join-builder.cc M be/src/exec/partitioned-hash-join-builder.h M be/src/exec/partitioned-hash-join-node-ir.cc M be/src/exec/partitioned-hash-join-node.cc M be/src/exec/partitioned-hash-join-node.h 5 files changed, 548 insertions(+), 372 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/14787/5 -- To view, visit http://gerrit.cloudera.org:8080/14787 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e233468de1eeae86651ab96df207de19e091053 Gerrit-Change-Number: 14787 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Tauber-Marshall
