[kudu-CR] master: additional leader lock assertions in catalog manager

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: master: additional leader lock assertions in catalog manager
..


Patch Set 6: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/3684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1303 Document code style guidelines for C++11 move semantics and rvalue references

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1303 Document code style guidelines for C++11 move 
semantics and rvalue references
..


Patch Set 1: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/3639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad1580a0fdb46c25af3af4fa61e250a0904b376a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Misty Stanley-Jones 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 4:

> Oh, apparently it needs a manual rebase (I guess due to the package
 > change)

Yeah, I'll take care of that.

-- 
To view, visit http://gerrit.cloudera.org:8080/3654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 4:

Oh, apparently it needs a manual rebase (I guess due to the package change)

-- 
To view, visit http://gerrit.cloudera.org:8080/3654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [java-client] Re-enable multi-master tests

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [java-client] Re-enable multi-master tests
..


Patch Set 4:

Adar, do you think we can push this now? I'll hit a rebase to make sure it 
still passes tests

-- 
To view, visit http://gerrit.cloudera.org:8080/3654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1051222738c84ef3d3e1a33b4981bc9454b7972
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] master: additional leader lock assertions in catalog manager

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: master: additional leader lock assertions in catalog manager
..


Patch Set 6:

Build Started http://104.196.14.100/job/kudu-gerrit/2714/

-- 
To view, visit http://gerrit.cloudera.org:8080/3684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] master: additional leader lock assertions in catalog manager

[Adar Dembo (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3684

to look at the new patch set (#6).

Change subject: master: additional leader lock assertions in catalog manager
..

master: additional leader lock assertions in catalog manager

I went through the catalog manager entry points and added leader lock
assertions where necessary, updating tests as needed.

I also snuck in a couple cluster fixes:
1. MiniCluster::leader_mini_master() was unsafe because it didn't pass the
   (now held) leader lock back to the caller. It's only used in a few places
   though, so I removed it outright rather than fix it.
2. WaitForTabletServerCount() has been updated for both kinds of clusters.
   The new version waits for the correct count on every master, a necessary
   change now that tservers heartbeat to every master. Without this, we may
   stop waiting when the master that has seen all tservers was a follower
   and fail a subsequent CreateTable. The new version also ignore masters
   that have been shut down. This isn't essential, but good future-proofing.

Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
---
M src/kudu/client/client-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/external_mini_cluster.h
M src/kudu/integration-tests/master_replication-itest.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.h
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master-path-handlers.cc
10 files changed, 155 insertions(+), 109 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/3684/6
-- 
To view, visit http://gerrit.cloudera.org:8080/3684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] remote bootstrap client: mild API changes

[Adar Dembo (Code Review)]

Adar Dembo has submitted this change and it was merged.

Change subject: remote_bootstrap_client: mild API changes
..


remote_bootstrap_client: mild API changes

1. I removed the uuid argument from the constructor; it's always the uuid of
   the FsManager.
2. I made the TabletStatusListener argument to FetchAll() optional. I think
   that was the original intent (because the TabletMetadata OUT parameter in
   Start() is optional), but a CHECK_NOTNULL() got added at some point.
3. I removed the peer uuid argument from Start(). It was only used for
   logging, so I added an equivalent field to the response protocol.

Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
Reviewed-on: http://gerrit.cloudera.org:8080/3811
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/integration-tests/remote_bootstrap-itest.cc
M src/kudu/tserver/remote_bootstrap.proto
M src/kudu/tserver/remote_bootstrap_client-test.cc
M src/kudu/tserver/remote_bootstrap_client.cc
M src/kudu/tserver/remote_bootstrap_client.h
M src/kudu/tserver/remote_bootstrap_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
7 files changed, 49 insertions(+), 52 deletions(-)

Approvals:
  Todd Lipcon: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/3811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-526: use on-disk cmeta when loading existing master state

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: KUDU-526: use on-disk cmeta when loading existing master state
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3786/3/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

Line 110:   if (master_->opts().IsDistributed()) {
> should we be verifying that, if the user passed any configuration on the co
That particular scenario can't happen: if there was a typo in a hostname the 
first time around, CreateDistributedConfig() would fail before any cmeta file 
is written to disk. Thus on restart, there'd be no cmeta to compare with. Of 
course, it's still a hard-to-understand state: the FS exists with no cmeta 
file, so the master would end up here in Load(), fail to load the cmeta file, 
and force the user to manually reformat.

In any case, I don't like the --master_addresses gflag because it only serves a 
purpose on first run and is confusing after that. We've established that using 
it to perform an offline config change is unsafe, so after the first run it has 
no value whatsoever. I think we should remove it and ask users to run a 
"format" command on each master, which would set up an FS and a cmeta without 
uuids. At runtime, each master would resolve the uuids with one another. We 
could even handle uuid resolution in "format" too:
1. On each master, format to create an FS. Get the uuid.
2. On each master, generate the cmeta using the set of all uuids and hostports.
3. Start each master. No uuid resolution needed.

It's a little more orchestration up front so I'm not sure if it's the best 
approach, but I like that it clearly delineates one-time admin setup from the 
work done by the masters at boot time.

Anyway, I'm curious to hear your thoughts no the above. I'm inclined to leave 
this as-is for now, unless you can think of a more plausible confusing failure 
scenario.


-- 
To view, visit http://gerrit.cloudera.org:8080/3786
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5b4c6d8b6adf696973445a6f9d1314ba9de27e70
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] master: additional leader lock assertions in catalog manager

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: master: additional leader lock assertions in catalog manager
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3684/5/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

Line 470:   // Return true if the table with the specified ID exists,
> doesn't return bool
Done


Line 473:   Status GetTableInfo(const std::string& table_id, 
scoped_refptr *table);
> should update docs to explain bad status results
Done


Line 477:   Status GetAllTables(std::vector* tables);
> same here
Done


Line 479:   // Return true if the specified table name exists
> same
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5bb2f5067cdbdd93900a80255def65a26216f6ea
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Predicate evaluation pushdown

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Predicate evaluation pushdown
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3841/1/src/kudu/cfile/binary_dict_block.cc
File src/kudu/cfile/binary_dict_block.cc:

Line 390: SeekAtOrAfterDictValue(ctx->pred().raw_lower(), _exact, 
lower_codeword);
could you change the API so that this only happens once when you construct the 
decoder, rather than once per row batch? That way you amortize all this "setup" 
cost across the entire cfile, instead of once per 1000 records, and should see 
a much bigger speedup


-- 
To view, visit http://gerrit.cloudera.org:8080/3841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6298defd710bc7badcc8f390d933b5b8613974e0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] remote bootstrap client: mild API changes

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: remote_bootstrap_client: mild API changes
..


Patch Set 4: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/3811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id7f482d1193a5a46fa51488eab4712c0c4bfceb1
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [C++ client] doxygenized all C++ client API

[Alexey Serbin (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3840

to look at the new patch set (#2).

Change subject: [C++ client] doxygenized all C++ client API
..

[C++ client] doxygenized all C++ client API

Doxygenized the rest of header files distributed along with
Kudu C++ client bundle.  The client.h file was already doxygenized.

Change-Id: Id0f82a6c8a500a892bc1daff8444e91191dab3af
---
M src/kudu/client/callbacks.h
M src/kudu/client/client.h
M src/kudu/client/resource_metrics.h
M src/kudu/client/row_result.h
M src/kudu/client/scan_batch.h
M src/kudu/client/scan_predicate.h
M src/kudu/client/schema.h
M src/kudu/client/shared_ptr.h
M src/kudu/client/stubs.h
M src/kudu/client/value.h
M src/kudu/client/write_op.h
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/slice.h
M src/kudu/util/status.h
15 files changed, 1,270 insertions(+), 392 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/3840/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3840
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id0f82a6c8a500a892bc1daff8444e91191dab3af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [C++ client] doxygenized all C++ client API

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: [C++ client] doxygenized all C++ client API
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/2713/

-- 
To view, visit http://gerrit.cloudera.org:8080/3840
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f82a6c8a500a892bc1daff8444e91191dab3af
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Predicate evaluation pushdown

[Andrew Wong (Code Review)]

Andrew Wong has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3841

Change subject: Predicate evaluation pushdown
..

Predicate evaluation pushdown

This patch is a work in progress.

The premise of this patch is to avoid the excessive use of CPU when
evaluating column predicates in specific cases. Dictionary blocks,
for instance, can evaluate predicates by comparing codewords (UINT32)
rather than doing string comparisons.

See the design-doc for predicate-eval-pushdown for a brief overview of the
implementations.  This patch uses the sorted dictionary approach. Evaluation
via set can be found here: https://github.com/anjuwong/kudu/tree/pred-pushdown

Change-Id: I6298defd710bc7badcc8f390d933b5b8613974e0
---
A docs/design-docs/predicate-eval-pushdown.md
M src/kudu/cfile/binary_dict_block.cc
M src/kudu/cfile/binary_dict_block.h
M src/kudu/cfile/block_encodings.h
M src/kudu/cfile/cfile_reader.cc
M src/kudu/cfile/cfile_reader.h
M src/kudu/codegen/CMakeLists.txt
A src/kudu/common/column_eval_context.h
M src/kudu/common/generic_iterators-test.cc
M src/kudu/common/generic_iterators.cc
M src/kudu/common/generic_iterators.h
M src/kudu/common/iterator.h
M src/kudu/common/schema.h
A src/kudu/scripts/plot_pred_pushed_times.py
A src/kudu/scripts/plot_pred_pushed_times.sh
A src/kudu/scripts/plot_times.py
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/delta_applier.cc
M src/kudu/tablet/delta_applier.h
M src/kudu/tablet/memrowset.cc
M src/kudu/tablet/memrowset.h
A src/kudu/tablet/tablet-decoder-eval-test.cc
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
28 files changed, 898 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/41/3841/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3841
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6298defd710bc7badcc8f390d933b5b8613974e0
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Andrew Wong 

[kudu-CR] [C++ client] doxygenized all C++ client API

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3840

Change subject: [C++ client] doxygenized all C++ client API
..

[C++ client] doxygenized all C++ client API

Doxygenized the rest of header files distributed along with
Kudu C++ client bundle.  The client.h file was already doxygenized.

Change-Id: Id0f82a6c8a500a892bc1daff8444e91191dab3af
---
M src/kudu/client/callbacks.h
M src/kudu/client/client.h
M src/kudu/client/resource_metrics.h
M src/kudu/client/row_result.h
M src/kudu/client/scan_batch.h
M src/kudu/client/scan_predicate.h
M src/kudu/client/schema.h
M src/kudu/client/shared_ptr.h
M src/kudu/client/stubs.h
M src/kudu/client/value.h
M src/kudu/client/write_op.h
M src/kudu/common/partial_row.h
M src/kudu/util/monotime.h
M src/kudu/util/slice.h
M src/kudu/util/status.h
15 files changed, 1,245 insertions(+), 388 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/40/3840/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3840
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id0f82a6c8a500a892bc1daff8444e91191dab3af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 

[kudu-CR] [C++ client] doxygenized all C++ client API

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: [C++ client] doxygenized all C++ client API
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2711/

-- 
To view, visit http://gerrit.cloudera.org:8080/3840
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id0f82a6c8a500a892bc1daff8444e91191dab3af
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No