[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. The comments on https://issues.apache.org/jira/browse/LEGAL-64 seem to indicate that Artistic License might not be allowed in ASF projects. Bootstrap distributes it, and Bootstrap itself is MIT-licensed, but that doesn't mean ASF projects can distribute npm.js. Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Reviewed-on: http://gerrit.cloudera.org:8080/4438 Reviewed-by: Jim Apple Tested-by: Internal Jenkins --- M LICENSE.txt M bin/rat_exclude_files.txt D tests/comparison/leopard/static/js/npm.js M tests/comparison/leopard/templates/custom_run.template M tests/comparison/leopard/templates/index.template M tests/comparison/leopard/templates/report.template D www/bootstrap/js/npm.js 7 files changed, 70 insertions(+), 31 deletions(-) Approvals: Jim Apple: Looks good to me, approved Internal Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables
..
Patch Set 1:
(31 comments)
Ok, part 3. I have one more batch left...
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/catalog/KuduTable.java
File fe/src/main/java/com/cloudera/impala/catalog/KuduTable.java:
PS1, Line 222:
:
:
:
:
:
:
was this moved somewhere else? Maybe I'll find it as I keep reading...
PS1, Line 238: // Update the HMS
: if (reuseMetadata) {
: try {
: client.alter_table(msTable_.getDbName(),
msTable_.getTableName(), msTable_);
: } catch (TException e) {
: throw new TableLoadingException(e.getMessage());
: }
: }
Same deal, I'm not sure why reuseMetadata means update the HMS.
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:
PS1, Line 220:
: private final static KuduCatalogOpExecutor kuduExecutor_
: = new KuduCatalogOpExecutor();
all static methods, why instantiate it?
PS1, Line 1122: // The db == null case isn't handled. The only tables this
should matter for are
: // Kudu tables. The expectation is Impala will always know
about any Kudu tables
: // because Hive doesn't support Kudu yet.
I've always had a problem with this comment :/ even after trimming it down I
don't think it makes complete sense. This should mention briefly what this code
_is handling_ before it talks about what it's not handling. Can you add 1
sentence at the beginning?
PS1, Line 1124: When Hive supports Kudu the DDL delegates
: // can be removed.
https://issues.cloudera.org/browse/IMPALA-3424 tracks the removal.
Can you remove these sentences and just leave IMPALA-3424 as a reference? We
may not remove our kudu code even if/when Hive supports Kudu.
PS1, Line 1127: ,
nit: extra comma
PS1, Line 1150: // The operation will be aborted if the Kudu table
cannot be dropped. If for
: // some reason Kudu is permanently stuck in a
non-functional state, the user is
: // expected to ALTER TABLE to either set the table to
UNMANAGED or set the format
: // to something else.
JIRA? Even if we don't fix it we should have something public to point to
regarding this behavior. Also to make sure the docs team knows to document it.
PS1, Line 1205: kuduExecutor_
static ref
PS1, Line 1406: if (KuduTable.isKuduTable(tbl)) {
: return createKuduTable(tbl, params.if_not_exists,
params.getDistribute_by(),
: response);
: }
: return createTable(tbl, params.if_not_exists,
params.getCache_op(), response);
: }
it's too bad we have to just branch like this and handle everything
differently, but I can't think of anything better.
PS1, Line 1420: createMetaStoreTable
I'd prefer to have this on the prev line and the parameter on the new line
Line 1475:* creation of a managed Kudu table.
comment on response param
PS1, Line 1480: TDdlExecResponse response)
I think this just fits on the prev line
PS1, Line 1483: kuduExecutor_
static reference KuduCatalogOpExecutor, here elsewhere in this fn
PS1, Line 1509: // Add the table to the catalog cache
: Table newTbl = catalog_.addTable(newTable.getDbName(),
newTable.getTableName());
: addTableToCatalogUpdate(newTbl, response.result);
While I don't think these will throw, it might be worth wrapping all the logic
after the Kudu create table in a try { } catch block that drops the kudu table.
That'll future proof this a bit. Feel free to ignore though.
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/service/Frontend.java
File fe/src/main/java/com/cloudera/impala/service/Frontend.java:
PS1, Line 230: impaladCatalog_.getDefaultKuduMasterAddrs()
this is fine but a little weird, normally I'm all about removing extra state
but in this case it might be better to keep this as a parameter on frontend and
pass it in. Unless there's somewhere else besides the old impaladCatalog we can
get it?
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/service/JniCatalog.java
File fe/src/main/java/com/cloudera/impala/service/JniCatalog.java:
PS1, Line 83:
: int otherLogLevel, boolean allowAuthToLocal, String
kerberosPrincipal)
wrapping
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/servi
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Jim Apple has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4014: HEADERS ONLY: Introduce query-wide execution state.
Alex Behm has posted comments on this change.
Change subject: IMPALA-4014: HEADERS ONLY: Introduce query-wide execution state.
..
Patch Set 1:
(12 comments)
Mostly clarifying questions. Sorry if Henry and you had already reached
consensus on some of these.
http://gerrit.cloudera.org:8080/#/c/4418/1/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:
Line 27: /// Tear-down happens automatically in the d'tor and frees all memory
allocated for
Didn't we want to move away from implicit destruction? Why is it ok to rely on
that here?
Line 31: /// ReportProfile() is invoked periodically to report the execution
status. The
No ReportProfile() in here. Wouldn't the reporting be controlled by the
QueryState by periodically collecting and aggregating the profile() from all
fragment instance states?
Line 52: /// It is an error to delete a FragmentInstanceState before
Open()/GetNext() (depending
What if Prepare() fails?
Line 94: Status Open();
Given my comment below, should this be Exec() or similar?
Line 100: Status GetNext(RowBatch** batch);
What's this call for? Don't all fragment instances end in a sink? I know that
today the coord fragment is an exception, but that's going to change.
Line 120: void ReleaseThreadToken();
When is this token acquired exactly?
http://gerrit.cloudera.org:8080/#/c/4418/1/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
Line 24: /// Central class for all backend execution state created for a
particular query,
This first paragraph could use some cleanup, e.g., grammar of the first
sentence is weird, "such as" seems misplaced. Also there seems to be some
redundancy with the FragmentInstanceState explanations.
Line 30: /// The lifetime of an instance of this class is dictated by a
reference count.
This sounds much like a shared_ptr, i.e., implicit management of the lifetime.
What is the motivation for this design? It does not seem to prevent certain
interesting races with e.g., RPCs wanting to access the query state after other
threads are done with it (has transitioned to a 0 ref count).
Incoming RPCs for data exchanges seem especially interesting, e.g. when there
is a limit somewhere downstream that could cause cancellation.
Line 41: class Guard {
ScopedQueryState? This makes it clear that the access is scoped, and we already
have similarly names classes such as ScopedSessionState.
Line 66: /// Illegal to call any other function of this class after this call
returns.
When is this legal to call with respect to the reference count? Will this call
block until reference count has reached 0?
Line 76: Status PublishFilter(const TUniqueId& dst_finstance_id, int32_t
filter_id,
Isn't a filter really published to a plan node contained in all relevant
fragment instances? In other words, why not have:
PublishFilter(TPlanNodeId dst_plan_node_id, int32_t filter_id, const
TBloomFilter& thrift_bloom_filter);
That way we'll only have to deserialize the thrift_bloom_filter once and we can
shield callers from knowing how many target fragment instances there are.
Another thought: Aren't runtime filters really shared among all relevant
fragment instances? It might make more sense to have the RuntimeFilterBanks in
here instead of the FragmentInstanceStates. Are we worried about exploding the
memory consumption of runtime filters by DOP?
Line 78: };
We'll deal with local filter aggregation separately correct?
--
To view, visit http://gerrit.cloudera.org:8080/4418
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Henry Robinson
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Matthew Jacobs
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4008: Don't bake ExprContext pointers into IR code
Michael Ho has posted comments on this change. Change subject: IMPALA-4008: Don't bake ExprContext pointers into IR code .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/4390/3/be/src/exec/hash-table.cc File be/src/exec/hash-table.cc: PS3, Line 164: std:: will remove std:: -- To view, visit http://gerrit.cloudera.org:8080/4390 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I42039eed803a39fa716b9ed647510b6440974ae5 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4008: Don't bake ExprContext pointers into IR code
Michael Ho has uploaded a new patch set (#3). Change subject: IMPALA-4008: Don't bake ExprContext pointers into IR code .. IMPALA-4008: Don't bake ExprContext pointers into IR code To allow genearated code to be shared across multiple fragment instances, this change removes the ExprContext pointers baked into various IR functions (e.g. AGG/PAGG/hash-table). Change-Id: I42039eed803a39fa716b9ed647510b6440974ae5 --- M be/src/codegen/gen_ir_descriptions.py M be/src/codegen/llvm-codegen.cc M be/src/codegen/llvm-codegen.h M be/src/exec/aggregation-node-ir.cc M be/src/exec/aggregation-node.cc M be/src/exec/aggregation-node.h M be/src/exec/hash-table-ir.cc M be/src/exec/hash-table.cc M be/src/exec/hash-table.h M be/src/exec/partitioned-aggregation-node-ir.cc M be/src/exec/partitioned-aggregation-node.cc M be/src/exec/partitioned-aggregation-node.h M be/src/exprs/agg-fn-evaluator.h 13 files changed, 345 insertions(+), 182 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/4390/3 -- To view, visit http://gerrit.cloudera.org:8080/4390 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I42039eed803a39fa716b9ed647510b6440974ae5 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4008: Don't bake ExprContext pointers into IR code
Michael Ho has posted comments on this change. Change subject: IMPALA-4008: Don't bake ExprContext pointers into IR code .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/4390/2/be/src/exec/aggregation-node.cc File be/src/exec/aggregation-node.cc: Line 84: // Use NULL if the aggregate evaluator is not codegen'ed or if there is no > It might be simpler just to always set to NULL if input_expr_ctxs is empty. That sounds reasonable. It may be simpler to just check if evaluator->input_expr_ctxs() == 1. That's the case for all aggregate evaluator which we codegen today. We may have more than one expr_ctxs_ for some evaluators (e.g. group_concat) but we don't codegen them. http://gerrit.cloudera.org:8080/#/c/4390/2/be/src/exec/aggregation-node.h File be/src/exec/aggregation-node.h: Line 82: std::vector agg_expr_ctxs_; > Mention that the ExprContexts are owned by aggregate_evaluators_. Done http://gerrit.cloudera.org:8080/#/c/4390/2/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: Line 158: ExprContext* agg_expr_ctx = NULL; > Again, I think it would be simpler to just check if it's empty. Or add a me Please see replies elsewhere. http://gerrit.cloudera.org:8080/#/c/4390/2/be/src/exec/partitioned-aggregation-node.h File be/src/exec/partitioned-aggregation-node.h: Line 202: /// ExprContexts of 'aggregate_evaluators_'. Used in the codegen'ed version of > Also document ownership here. Done Line 205: std::vector agg_expr_ctxs_; > What do you think about making this vectorhttp://gerrit.cloudera.org:8080/#/c/4390/2/be/src/exprs/agg-fn-evaluator.h File be/src/exprs/agg-fn-evaluator.h: Line 192: std::vector input_expr_ctxs_; > Can you add your comment here about when this is empty/non-empty? Done -- To view, visit http://gerrit.cloudera.org:8080/4390 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I42039eed803a39fa716b9ed647510b6440974ae5 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: Verified-1 Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/213/ -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4014: HEADERS ONLY: Introduce query-wide execution state.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-4014: HEADERS ONLY: Introduce query-wide execution state.
..
Patch Set 1:
(17 comments)
http://gerrit.cloudera.org:8080/#/c/4418/1/be/src/runtime/fragment-instance-state.h
File be/src/runtime/fragment-instance-state.h:
PS1, Line 32: setting
: /// that flag to 0 disables periodic reporting altogether.
Possibly a good time to retire that functionality which I don't think is ever
used.
PS1, Line 43: TODO:
: /// - move tear-down logic out of d'tor and into TearDown()
function
Why can't we do that in this patch (given it's header only...)?
PS1, Line 56: QueryState* query_state() { return query_state_; }
When would we want to get a mutable query_state() from its FIS? That
circumstance would suggest the caller violated the top-down pattern for getting
at the FIS in the first place.
PS1, Line 69: /// Set the execution thread, taking ownership of the object.
: void set_exec_thread(Thread* exec_thread) {
exec_thread_.reset(exec_thread); }
I do find this API weird, always have. Why doesn't this object start its own
thread?
PS1, Line 116: /// Returns true if this query has a limit and it has been
reached.
: bool ReachedLimit();
FYI, this and GetNext() are likely to be removed with my patch for IMPALA-2905
(subject to review). Might be worth anticipating that here.
http://gerrit.cloudera.org:8080/#/c/4418/1/be/src/runtime/query-exec-mgr.h
File be/src/runtime/query-exec-mgr.h:
PS1, Line 24: an entry point for
: /// execution-related rpcs (ExecPlanFragment/CancelPlanFragment)
I don't think this is relevant - we shouldn't comment on what might call this
class.
PS1, Line 33: Also creates a QueryState for this query, if none exists, and
sets its refcount
: /// to 1. If a QueryState already exists for this query,
increments the refcount.
Not clear from this who owns that reference, and who is therefore responsible
for releasing it.
PS1, Line 43: /// Cancels a particular fragment instance.
: Status CancelFInstance(const TUniqueId& finstance_id);
Why is this here, and not accessible through
GetQueryState(query_id)->Cancel(...) ?
PS1, Line 55: PublishFilter
Again, don't see that this needs to be in this class.
http://gerrit.cloudera.org:8080/#/c/4418/1/be/src/runtime/query-state.h
File be/src/runtime/query-state.h:
PS1, Line 31: threads
too specific: clients of this class must obtain a reference.
PS1, Line 41: class Guard {
comment on usage pattern, which is going to be common. Particularly that
callers must check if query_state() is not null, and if it is not null then it
will live as long as the guard.
FWIW, I prefer ScopedRef as a name. The 'guard' in lock_guard<> means - I think
- that the object guards a critical section by preventing concurrent access.
PS1, Line 44: (ExecEnv::GetQueryExecMgr()
shouldn't there be corresponding changes to ExecEnv then? Or are you leaving
those out to make this self-contained?
Either way - prefer to do ExecEnv::GetInstance()->query_exec_mgr(). Under what
circumstances would this ever be null?
PS1, Line 49: DCHECK(ExecEnv::GetQueryExecMgr() != nullptr);
I don't think we need this, but presumably it's redundant here if the
constructor checked it exists.
PS1, Line 60: query
nit picking here, but it might be helpful to distinguish the query lifetime
from the lifetime of the set of fragment instances; since the former can be
significantly longer than the latter. Not sure what a good alternative is
without introducing a new phrase for a group of fragment instances.
PS1, Line 65: Delete all query state in this class or accessible through this
class.
Would just say it releases all resources owned by this class ("accessible
through this class" sounds like it could reach out and delete other structures'
data).
PS1, Line 69: /// Registers a new FInstanceState.
Either the comment or the method name should change. I would change the comment
- the fact that an FInstanceState is registered seems like an implementation
detail.
PS1, Line 73: Status
Under what conditions would this return an error?
--
To view, visit http://gerrit.cloudera.org:8080/4418
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I962ae6b7cb7dc0d07fbb8f70317aeb01d88d400b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Marcel Kornacker
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Henry Robinson
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Matthew Jacobs
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4138: Fix AcquireState() for batches that change capacity
Henry Robinson has posted comments on this change. Change subject: IMPALA-4138: Fix AcquireState() for batches that change capacity .. Patch Set 4: (2 comments) Rebased to include IMPALA_ASSERT_DEBUG_DEATH macro from trunk. http://gerrit.cloudera.org:8080/#/c/4428/4//COMMIT_MSG Commit Message: PS4, Line 11: this is usually calculated from : capacity() > are there places that incorrectly use capacity() that should be changed to I have one in my upcoming patch for IMPALA-2905. Independently, this seems like a shortcoming in the API - there's no way to do AcquireState() after capacity changes without sharing knowledge of the initial capacity through a separate channel. Otherwise there are a couple of AcquireState() calls, but they use row batches that have been initialized with RuntimeState::batch_size() rows. http://gerrit.cloudera.org:8080/#/c/4428/4/be/src/runtime/row-batch.h File be/src/runtime/row-batch.h: PS4, Line 360: capacity_ > not due to your change, but this comment should be fixed. Done -- To view, visit http://gerrit.cloudera.org:8080/4428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6ceca53c406b05cd04b7d95a4f9f2eec7bc127f5 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4138: Fix AcquireState() for batches that change capacity
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4428 to look at the new patch set (#5). Change subject: IMPALA-4138: Fix AcquireState() for batches that change capacity .. IMPALA-4138: Fix AcquireState() for batches that change capacity If MarkAtCapacity() is called on a row batch, it is difficult to call AcquireState() on that batch because tuple_ptrs_size_ is not accessible to initialise the destination batch - this is usually calculated from capacity(), but that value is wrong for these purposes after MarkAtCapacity(). Add RowBatch::InitialCapacity() to return the initial capacity value of the batch. Add row-batch-test to add initial coverage of AcquireState() API. Change-Id: I6ceca53c406b05cd04b7d95a4f9f2eec7bc127f5 --- M be/src/runtime/CMakeLists.txt A be/src/runtime/row-batch-test.cc M be/src/runtime/row-batch.cc M be/src/runtime/row-batch.h 4 files changed, 78 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/4428/5 -- To view, visit http://gerrit.cloudera.org:8080/4428 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6ceca53c406b05cd04b7d95a4f9f2eec7bc127f5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1654: General partition exprs in DDL operations.
Jim Apple has posted comments on this change. Change subject: IMPALA-1654: General partition exprs in DDL operations. .. Patch Set 13: > (1 comment) > > Thanks for helping me fix the auditing bugs. I wish this wiki may > help me get my local env working. > https://cwiki.apache.org/confluence/display/IMPALA/How+to+load+and+run+Impala+tests I have updated that page, now, with some extra information. -- To view, visit http://gerrit.cloudera.org:8080/3942 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Amos Bird Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Amos Bird Gerrit-Reviewer: Jim Apple Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 1)
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 1) .. Patch Set 3: I have finally gotten the BVTs to successfully run. Packaging run: http://golden.jenkins.cloudera.com/job/CDH5-Packaging-On-Demand/3043/ That failed due to a timeout while waiting for the BVT job, but the BVT job itself ran and succeeded: http://qe.jenkins.cloudera.com/job/docker-build_cdh_cluster/957/ and the packaging job doesn't have any significant steps after the BVT job, so if it hadn't timed out everything would have succeeded. -- To view, visit http://gerrit.cloudera.org:8080/3936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3767dd1ee86df767075fdf1b371eb6b0b06668db Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1616: Improve the Memory Limit Exceeded error report
Michael Ho has posted comments on this change. Change subject: IMPALA-1616: Improve the Memory Limit Exceeded error report .. Patch Set 5: Code-Review+2 Carry +2 forward. -- To view, visit http://gerrit.cloudera.org:8080/4335 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: Code-Review+2 Looks good to me. -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Michael Brown has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: Code-Review+1 > Let me run the Leopard front end and do a sanity test. It's fine. +1 for the Leopard changes. -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4136: testKudu planner test hangs if Kudu is not supported
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4136: testKudu planner test hangs if Kudu is not supported .. IMPALA-4136: testKudu planner test hangs if Kudu is not supported The Kudu planner tests were recently moved into PlannerTest but they need to be explicitly skipped when KUDU_IS_SUPPORTED is false. Change-Id: Iaf713107cdf49fa73efae5a731f26bbce96d8364 Reviewed-on: http://gerrit.cloudera.org:8080/4426 Reviewed-by: Alex Behm Tested-by: Internal Jenkins --- M fe/src/test/java/com/cloudera/impala/planner/PlannerTest.java 1 file changed, 19 insertions(+), 4 deletions(-) Approvals: Internal Jenkins: Verified Alex Behm: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Iaf713107cdf49fa73efae5a731f26bbce96d8364 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Internal Jenkins
[Impala-ASF-CR] IMPALA-4136: testKudu planner test hangs if Kudu is not supported
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4136: testKudu planner test hangs if Kudu is not supported .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf713107cdf49fa73efae5a731f26bbce96d8364 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Matthew Jacobs Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Internal Jenkins Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Michael Brown has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: Let me run the Leopard front end and do a sanity test. -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Jim Apple has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4438/1/www/bootstrap/js/npm.js File www/bootstrap/js/npm.js: Line 2 > Yes, exactly. Check that the webpage works on the local minicluster. I woul OK, tested. Looks fine to me. -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database
Michael Brown has posted comments on this change. Change subject: IMPALA-3980: qgen: re-enable Hive as a target database .. Patch Set 7: Code-Review+1 This needs a committer's look. -- To view, visit http://gerrit.cloudera.org:8080/4011 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: stak...@cloudera.com Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: stak...@cloudera.com Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4438/1/www/bootstrap/js/npm.js File www/bootstrap/js/npm.js: Line 2 > No. How would you suggest I test - maybe start up Impala minicluster and lo Yes, exactly. Check that the webpage works on the local minicluster. I would click on a few buttons to make sure everything is in order. -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Jim Apple has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4438/1/www/bootstrap/js/npm.js File www/bootstrap/js/npm.js: Line 2 > Did you check that the Impala webpage still works after deleting this file? No. How would you suggest I test - maybe start up Impala minicluster and load the basic webpage that appears on localhost:25000 (or whatever port it is)? -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4438/1/www/bootstrap/js/npm.js File www/bootstrap/js/npm.js: Line 2 Did you check that the Impala webpage still works after deleting this file? -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3786: Replace "cloudera" with "apache" (part 1)
Jim Apple has posted comments on this change. Change subject: IMPALA-3786: Replace "cloudera" with "apache" (part 1) .. Patch Set 3: Any news on this? -- To view, visit http://gerrit.cloudera.org:8080/3936 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3767dd1ee86df767075fdf1b371eb6b0b06668db Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1670: Support multiple partitions in ALTER TABLE ADD PARTITION
Attila Jeges has posted comments on this change. Change subject: IMPALA-1670: Support multiple partitions in ALTER TABLE ADD PARTITION .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/4144/8/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: PS8, Line 1766: :* Drops an existing partition from the given table in Hive. If the partition is cached, :* the associated cache directive will also be removed. :* Also drops the partition from its Hdfs table. :* Re > No, I don't think is the correct behavior. Actually I filed a JIRA (IMPALA- Makes sense, thank you for clarifying it. -- To view, visit http://gerrit.cloudera.org:8080/4144 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iddbc951f2931f488f7048c9780260f6b49100750 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database
stak...@cloudera.com has posted comments on this change. Change subject: IMPALA-3980: qgen: re-enable Hive as a target database .. Patch Set 7: > > The hive-default.xml file under > > fe/src/test/resources/ is not a valid XML file. It was missing > some > > element terminators. Once I fixed those the tests starting > working. > > I have updated the patch. > > Thanks, can you update the commit message as well? Updated -- To view, visit http://gerrit.cloudera.org:8080/4011 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: stak...@cloudera.com Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: stak...@cloudera.com Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database
stak...@cloudera.com has uploaded a new patch set (#7). Change subject: IMPALA-3980: qgen: re-enable Hive as a target database .. IMPALA-3980: qgen: re-enable Hive as a target database Changes: * Added hive cli options back in (removed in commit "Stress test: Various changes") * Modifications so that if --use-hive is specified, a Hive connection is actually created * A few minor bug fixes so that the RQG can be run locally * Modified MiniCluster to use HADOOP_CONF_DIR and HIVE_CONF_DIR rather than a hard-coded file under IMPALA_HOME * Fixed fe/src/test/resources/hive-default.xml so that it is a valid XML file, it was missing a few element terminators that cause an exception in the cluster.py file Testing: * Hive integration tested locally by invoking the data generator via the command: ./data-generator.py \ --db-name=functional \ --use-hive \ --min-row-count=50 \ --max-row-count=100 \ --storage-file-formats textfile \ --use-postgresql \ --postgresql-user stakiar and the discrepancy checker via the command: ./discrepancy-checker.py \ --db-name=functional \ --use-hive \ --use-postgresql \ --postgresql-user stakiar \ --test-db-type HIVE \ --timeout 300 \ --query-count 50 \ --profile hive * The output of the above two commands is essentially the same as the Impala output, however, about 20% of the queries will fail when the discrepancy checker is run * Regression testing done by running Leopard in a local VM running Ubuntu 14.04, and by running the discrepancy checker against Impala while inside an Impala Docker container Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 --- M fe/src/test/resources/hive-default.xml M tests/comparison/cli_options.py M tests/comparison/cluster.py M tests/comparison/data_generator.py 4 files changed, 66 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/4011/7 -- To view, visit http://gerrit.cloudera.org:8080/4011 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: stak...@cloudera.com Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: stak...@cloudera.com
[Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database
Michael Brown has posted comments on this change. Change subject: IMPALA-3980: qgen: re-enable Hive as a target database .. Patch Set 6: > The hive-default.xml file under > fe/src/test/resources/ is not a valid XML file. It was missing some > element terminators. Once I fixed those the tests starting working. > I have updated the patch. Thanks, can you update the commit message as well? -- To view, visit http://gerrit.cloudera.org:8080/4011 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: stak...@cloudera.com Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: stak...@cloudera.com Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database
stak...@cloudera.com has posted comments on this change. Change subject: IMPALA-3980: qgen: re-enable Hive as a target database .. Patch Set 6: > Uploaded patch set 6. Hey Michael, Apologies, yes you are correct, the tests were failing with this patch. It seems my dev setup wasn't fully setup properly. Originally, I was trying to avoid building Impala fully, so I made some modifications to get the Python code to run locally. The tests were passing locally, but I realize this probably isn't a great approach if I want to reliably run these unit tests. I've set up a proper Impala dev environment on a remote machine. Once I did that I was able to re-produce the error. The hive-default.xml file under fe/src/test/resources/ is not a valid XML file. It was missing some element terminators. Once I fixed those the tests starting working. I have updated the patch. -- To view, visit http://gerrit.cloudera.org:8080/4011 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: stak...@cloudera.com Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: stak...@cloudera.com Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4116: Remove 'cdh' from version string again
Jim Apple has submitted this change and it was merged. Change subject: IMPALA-4116: Remove 'cdh' from version string again .. IMPALA-4116: Remove 'cdh' from version string again The rebase of another change went wrong and undid the change of I7754538a23e73dcdebc6e3df509f357cbe03198c so we need to do this one again. Previous review was at http://gerrit.cloudera.org:8080/4421 . Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Reviewed-on: http://gerrit.cloudera.org:8080/4439 Reviewed-by: Sailesh Mukil Reviewed-by: Lars Volker Reviewed-by: Jim Apple Tested-by: Sailesh Mukil --- M bin/save-version.sh 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jim Apple: Looks good to me, approved Lars Volker: Looks good to me, but someone else must approve Sailesh Mukil: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/4439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil
[Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database
stak...@cloudera.com has uploaded a new patch set (#6). Change subject: IMPALA-3980: qgen: re-enable Hive as a target database .. IMPALA-3980: qgen: re-enable Hive as a target database Changes: * Added hive cli options back in (removed in commit "Stress test: Various changes") * Modifications so that if --use-hive is specified, a Hive connection is actually created * A few minor bug fixes so that the RQG can be run locally * Modified MiniCluster to use HADOOP_CONF_DIR and HIVE_CONF_DIR rather than a hard-coded file under IMPALA_HOME Testing: * Hive integration tested locally by invoking the data generator via the command: ./data-generator.py \ --db-name=functional \ --use-hive \ --min-row-count=50 \ --max-row-count=100 \ --storage-file-formats textfile \ --use-postgresql \ --postgresql-user stakiar and the discrepancy checker via the command: ./discrepancy-checker.py \ --db-name=functional \ --use-hive \ --use-postgresql \ --postgresql-user stakiar \ --test-db-type HIVE \ --timeout 300 \ --query-count 50 \ --profile hive * The output of the above two commands is essentially the same as the Impala output, however, about 20% of the queries will fail when the discrepancy checker is run * Regression testing done by running Leopard in a local VM running Ubuntu 14.04, and by running the discrepancy checker against Impala while inside an Impala Docker container Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 --- M fe/src/test/resources/hive-default.xml M tests/comparison/cli_options.py M tests/comparison/cluster.py M tests/comparison/data_generator.py 4 files changed, 66 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/4011/6 -- To view, visit http://gerrit.cloudera.org:8080/4011 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: stak...@cloudera.com Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: stak...@cloudera.com
[Impala-ASF-CR] IMPALA-4116: Remove 'cdh' from version string again
Lars Volker has posted comments on this change. Change subject: IMPALA-4116: Remove 'cdh' from version string again .. Patch Set 1: As a non-committer I cannot submit this. Can one of you please submit and push it to the asf repo? Thanks a lot. -- To view, visit http://gerrit.cloudera.org:8080/4439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4116: Remove 'cdh' from version string again
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4116: Remove 'cdh' from version string again .. Patch Set 1: Verified+1 > Since this already passed GVO, I think it's OK to commit without > doing that again. Manually verifying as it already went through GVO once before. -- To view, visit http://gerrit.cloudera.org:8080/4439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4116: Remove 'cdh' from version string again
Jim Apple has posted comments on this change. Change subject: IMPALA-4116: Remove 'cdh' from version string again .. Patch Set 1: Code-Review+2 Since this already passed GVO, I think it's OK to commit without doing that again. -- To view, visit http://gerrit.cloudera.org:8080/4439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4116: Remove 'cdh' from version string again
Lars Volker has posted comments on this change. Change subject: IMPALA-4116: Remove 'cdh' from version string again .. Patch Set 1: Code-Review+1 No worries. I think we should merge this right away. The previous attempt was done after full GVO so I don't see why anything could be wrong now. -- To view, visit http://gerrit.cloudera.org:8080/4439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] Make gen build version.py resilient to a failing git rev-parse
Sailesh Mukil has posted comments on this change. Change subject: Make gen_build_version.py resilient to a failing git rev-parse .. Patch Set 8: > (1 comment) Sorry about that. -- To view, visit http://gerrit.cloudera.org:8080/4411 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7af33502bbb70185dc15ffca6219436a616f25b Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4116: Remove 'cdh' from version string again
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-4116: Remove 'cdh' from version string again .. Patch Set 1: Code-Review+2 > Uploaded patch set 1. Really sorry about this. This patch and my patch got committed together and I reverted this on rebase by mistake. -- To view, visit http://gerrit.cloudera.org:8080/4439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-ASF-CR] Make gen build version.py resilient to a failing git rev-parse
Lars Volker has posted comments on this change. Change subject: Make gen_build_version.py resilient to a failing git rev-parse .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/4411/8/bin/save-version.sh File bin/save-version.sh: Line 24: VERSION=2.8.0-cdh5-INTERNAL > Was this the conflict? I think maybe it should be rolled back to 2.7.0. I agree with Jim. Here's the change that fixes this: http://gerrit.cloudera.org:8080/4439 -- To view, visit http://gerrit.cloudera.org:8080/4411 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7af33502bbb70185dc15ffca6219436a616f25b Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4116: Remove 'cdh' from version string again
Lars Volker has uploaded a new change for review. http://gerrit.cloudera.org:8080/4439 Change subject: IMPALA-4116: Remove 'cdh' from version string again .. IMPALA-4116: Remove 'cdh' from version string again The rebase of another change went wrong and undid the change of I7754538a23e73dcdebc6e3df509f357cbe03198c so we need to do this one again. Previous review was at http://gerrit.cloudera.org:8080/4421 . Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f --- M bin/save-version.sh 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/39/4439/1 -- To view, visit http://gerrit.cloudera.org:8080/4439 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie386d25f2006e2dcebcbfd3d6ae88f70d65efb0f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker
[Impala-ASF-CR] IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-3719: Simplify CREATE TABLE statements with Kudu tables
..
Patch Set 1:
(17 comments)
another batch of comments, still a lot of files i haven't touched yet...
http://gerrit.cloudera.org:8080/#/c/4414/1/be/src/catalog/catalog.cc
File be/src/catalog/catalog.cc:
PS1, Line 47: {"", "(ZILjava/lang/String;IIZLjava/lang/String;)V",
: &catalog_ctor_},
1 line
http://gerrit.cloudera.org:8080/#/c/4414/1/be/src/service/frontend.cc
File be/src/service/frontend.cc:
PS1, Line 37: // XXX: This flag doesn't seem to be used anywhere. Maybe remove
it?
: DEFINE_bool(load_catalog_at_startup, false, "if true, load all
catalog data at startup");
agreed. doesn't look like it's used by JniFrontend. Let's remove it.
PS1, Line 63: IPs
IP addresses.
PS1, Line 63:
no space
http://gerrit.cloudera.org:8080/#/c/4414/1/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
PS1, Line 52: THdfsFileFormat
Maybe not for this patch since it's already huge, but it'd be great to
generalize this if we can. I can think of two improvements:
1) Maybe we should model the storage layer, e.g. have a TStorageEngine, then
make this TFileFormat (perhaps). This is probably a big change.
2) Rename this to be TStorageFormat, which kind of addresses #1 but doesn't
separate out storage engines and file formats.
PS1, Line 61: THdfsCompression
similarly, this seems unnecessarily specific to Hdfs. Not necessarily something
to change now but maybe we can create a follow-up JIRA to clean this up.
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
PS1, Line 31: ColumnDefOptions
this class doesn't exist, please add the missing file
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/analysis/DistributeParam.java
File fe/src/main/java/com/cloudera/impala/analysis/DistributeParam.java:
PS1, Line 84: BigDecimal
why BigDecimal? Ultimately this has to resolve to some int for kudu's API. (We
can check if it's 32 or 64bit).
PS1, Line 110: <= 1)
is 1 bucket actually not valid?
PS1, Line 138: colType.isStringType() && !exprType.isStringType()
: || colType.isIntegerType() &&
(!exprType.isIntegerType()
: || exprType.getPrecision() >
colType.getPrecision())
1. I don't see anything in the Kudu client that explicitly says you can't
partition on any particular types. This code will exclude boolean and floating
pt types, which is maybe unnecessary.
2. Esp if we can address #1, is there a cleaner way to make sure the types are
valid rather than enumerating the kinds of types to consider? I'm not sure, but
maybe one of the frontend gurus can think of something.
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/catalog/HdfsFileFormat.java
File fe/src/main/java/com/cloudera/impala/catalog/HdfsFileFormat.java:
PS1, Line 39: HdfsFileFormat
Can you open a JIRA (and leave it in the comment) to refactor this later?
http://gerrit.cloudera.org:8080/#/c/4414/1/fe/src/main/java/com/cloudera/impala/catalog/KuduTable.java
File fe/src/main/java/com/cloudera/impala/catalog/KuduTable.java:
PS1, Line 79:
: // TODO we should have something like
KuduConfig.getDefaultConfig()
do you know what this means / can it be removed now that we're adding default
master addrs?
Line 92: public static final String KEY_DISTRIBUTE_BY = "kudu.distribute_by";
comment
Line 153:* Load the columns from the schema list
can you add a comment about error handling in this function?
PS1, Line 158: LOG.error(String.format("Kudu tables must have at least
one"
: + "key column (had %d), and no more key columns than
there are table columns "
: + "(had %d).", keyColumns.size(), schema.size()));
shouldn't this still fail? if not, can you add a comment why this continues?
PS1, Line 184: LOG.error(String.format("Some key columns were not found
in"
: + " the set of columns. List of column names: %s,
List of key column names:"
: + " %s", Iterables.toString(columnNames),
Iterables.toString(keyColumns)));
why do we continue?
PS1, Line 199: // Get the table metadata from Kudu
: if (reuseMetadata) {
I'm confused about this. It's not clear to me from the name 'reuseMetadata' why
this means we should populate the metadata from Kudu. If anything, it sounds
like it would be the opposite.
The base class comment just says "If 'reuseMetadata' is true, reuse valid
existing metadata.".
--
To view, visit http://gerrit.cloudera.org:8080/4414
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7b9d51b2720
[Impala-ASF-CR] IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2.
Jim Apple has uploaded a new change for review. http://gerrit.cloudera.org:8080/4438 Change subject: IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. .. IMPALA-4110, IMPALA-3853: npm.js uses Artistic License 2. The comments on https://issues.apache.org/jira/browse/LEGAL-64 seem to indicate that Artistic License might not be allowed in ASF projects. Bootstrap distributes it, and Bootstrap itself is MIT-licensed, but that doesn't mean ASF projects can distribute npm.js. Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 --- M LICENSE.txt M bin/rat_exclude_files.txt D tests/comparison/leopard/static/js/npm.js M tests/comparison/leopard/templates/custom_run.template M tests/comparison/leopard/templates/index.template M tests/comparison/leopard/templates/report.template D www/bootstrap/js/npm.js 7 files changed, 70 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/38/4438/1 -- To view, visit http://gerrit.cloudera.org:8080/4438 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4bcbeeb6e8552894803315967b95f365227d7505 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Jim Apple
[Impala-ASF-CR] Make gen build version.py resilient to a failing git rev-parse
Jim Apple has posted comments on this change. Change subject: Make gen_build_version.py resilient to a failing git rev-parse .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/4411/8/bin/save-version.sh File bin/save-version.sh: Line 24: VERSION=2.8.0-cdh5-INTERNAL Was this the conflict? I think maybe it should be rolled back to 2.7.0. -- To view, visit http://gerrit.cloudera.org:8080/4411 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id7af33502bbb70185dc15ffca6219436a616f25b Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1616: Improve the Memory Limit Exceeded error report
Attila Jeges has uploaded a new patch set (#5). Change subject: IMPALA-1616: Improve the Memory Limit Exceeded error report .. IMPALA-1616: Improve the Memory Limit Exceeded error report The error report has been changed to include the id of the fragment instance that exceeded the memory limit. Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea --- M be/src/runtime/runtime-state.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/4335/5 -- To view, visit http://gerrit.cloudera.org:8080/4335 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1616: Improve the Memory Limit Exceeded error report
Hello Lars Volker, Michael Ho, Dan Hecht, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4335 to look at the new patch set (#5). Change subject: IMPALA-1616: Improve the Memory Limit Exceeded error report .. IMPALA-1616: Improve the Memory Limit Exceeded error report The error report has been changed to include the id of the fragment instance that exceeded the memory limit. Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea --- M be/src/runtime/runtime-state.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/4335/5 -- To view, visit http://gerrit.cloudera.org:8080/4335 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-1616: Improve the Memory Limit Exceeded error report
Attila Jeges has posted comments on this change. Change subject: IMPALA-1616: Improve the Memory Limit Exceeded error report .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4335/4/be/src/runtime/runtime-state.cc File be/src/runtime/runtime-state.cc: PS4, Line 250: this-> > please delete "this->". We generally only use that when there's an ambiguit Done -- To view, visit http://gerrit.cloudera.org:8080/4335 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibb4e0c359d889938b4c351771ba539850bdb95ea Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Attila Jeges Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. IMPALA-4111: backend death tests should not produce minidumps Move the existing core dump disabler into a shared utility header and also disable minidumps too. Add a macro to simplify use of the disabler. Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Reviewed-on: http://gerrit.cloudera.org:8080/4353 Reviewed-by: Tim Armstrong Tested-by: Internal Jenkins --- A be/src/testutil/death-test-util.h M be/src/util/minidump.cc M be/src/util/minidump.h M be/src/util/promise-test.cc 4 files changed, 83 insertions(+), 24 deletions(-) Approvals: Internal Jenkins: Verified Tim Armstrong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4111: backend death tests should not produce minidumps
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4111: backend death tests should not produce minidumps .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4353 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
