[kudu-CR] KUDU-1374: send full tablet report when new leader master is detected

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1374: send full tablet report when new leader master is 
detected
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3643/8/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

Line 405:   resp.leader_master()) {
nit: dont really need to check has_leader_master() in either of these cases -- 
it'll just default to false


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic16fc46736476dba39616e79ecfe79eee48b3d7f
Gerrit-PatchSet: 8
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] KUDU-1358 (part 3): new multi-master stress test

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1358 (part 3): new multi-master stress test
..


Patch Set 12: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
..


Patch Set 12: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..


Patch Set 12:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3609/12//COMMIT_MSG
Commit Message:

Line 20: possible; the only way a master can "lose" a cached TSDescriptor is if 
the
what about when a tserver restarts? is it still going to do a proper full TR 
after the restart, due to some tserver-side logic? are we waiting on the later 
patch in this series to make sure that the tserver logic ensures a full TR 
after a leader change?


http://gerrit.cloudera.org:8080/#/c/3609/12/src/kudu/integration-tests/mini_cluster.cc
File src/kudu/integration-tests/mini_cluster.cc:

Line 289: auto ts = mini_tablet_server->server();
should this verify that the TS process is running? (there's a getter for that). 
Worth documenting either way


Line 299:   }
else LOG(FATAL) perhaps? or use a switch() so that you get a compilation error 
if you were to add a new matchmode


http://gerrit.cloudera.org:8080/#/c/3609/12/src/kudu/integration-tests/mini_cluster.h
File src/kudu/integration-tests/mini_cluster.h:

Line 145: // Match the tservers retrieved from each master against the 
tservers in
not sure what 'Match' means here. Is this a verification? (i.e that the count 
and uuids match, and otherwise a bad Status will return?) Could be more clear.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] c++ client: use operation timeout as deadline for finding new leader master

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: c++ client: use operation timeout as deadline for finding new 
leader master
..


Patch Set 2:

Seems reasonable enough. The only potential concern is that I sort of recall 
picking the 'default RPC timeout' rather than the 'operation timeout' so that, 
if the master was actually down, the user would get an error quicker than their 
timeout, rather than a bunch of retries. If you try to contact a cluster which 
is just down, does it now hang for the full timeout? Or if we get 'connection 
refused' from all masters, does it bail out relatively quickly?

I suppose an argument could be made either way, but 'fast fail' seems to make 
sense at least for command-line interactive things if the cluster is actually 
down (not just a transient restart/failure)

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

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

[kudu-CR] ksck: improve filtering capability

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: ksck: improve filtering capability
..

ksck: improve filtering capability

- filters can now use glob-like pattern syntax
- filters now apply for the metadata checks, not just the checksums

Change-Id: Ic6ef8ab20679a9967c321cd4f8412ea4ea5fd50d
---
M src/kudu/integration-tests/cluster_verifier.cc
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/kudu-ksck.cc
6 files changed, 110 insertions(+), 60 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic6ef8ab20679a9967c321cd4f8412ea4ea5fd50d
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] ksck: improve output for long-running ksck checksums

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: ksck: improve output for long-running ksck checksums
..

ksck: improve output for long-running ksck checksums

Checksumming a large (multi-TB) table can take many minutes. Previously, the
ksck output would be very quiet during that time, giving no indication as to
whether it was making progress or how much work might be remaining.  This
addresses that by:

- passing back how many bytes and number of rows have been summed so
  far on a regular basis
- reporting progress every 5 seconds, including the above numbers

Along the way, I decided that our default timeout of 5 minutes was way too low
for typical table sizes, so bumped it to an hour. I also added more mock-based
test coverage of the checksum-scan code path.

Change-Id: I2a9962329570e8383087747d36cee9ad4fa60825
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_service.proto
9 files changed, 179 insertions(+), 61 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a9962329570e8383087747d36cee9ad4fa60825
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] ksck: fix a crash in checksum mode on tables with many tablets

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: ksck: fix a crash in checksum mode on tables with many tablets
..

ksck: fix a crash in checksum mode on tables with many tablets

In the case that the list of tablets had to be fetched in multiple
batches, we would improperly re-fetch the last tablet of the previous
batch as the first tablet of the next batch. This would then cause
a tablet to be inserted twice into the list, which would later cause
a CHECK failure when we tried to InsertOrDie() this tablet ID into
a map.

This fixes the issue by making sure that we look for more tablets starting with
the *successor* partition key compared to the previous tablet we fetched.
I also updated the integration test to use a table with more tablets
so that the batching code was exercised.

Change-Id: I4ca7ef75bd22ce27885e31ab20cf0e8e0ee2d355
---
M src/kudu/tools/ksck_remote-test.cc
M src/kudu/tools/ksck_remote.cc
2 files changed, 17 insertions(+), 10 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ca7ef75bd22ce27885e31ab20cf0e8e0ee2d355
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 

[kudu-CR](gh-pages) www: Replace old Kudu logo with new responsive Apache Kudu logo

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: www: Replace old Kudu logo with new responsive Apache Kudu logo
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3178b8197e78ffd7f439fba0fcc35c702381fe57
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] docs: Remove reference to old kudu-user mailing list

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: docs: Remove reference to old kudu-user mailing list
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2752b7988d960a24be1597906226996e7a920d4c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] ksck: report hostnames instead of IP addresses for tablet servers

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: ksck: report hostnames instead of IP addresses for tablet 
servers
..

ksck: report hostnames instead of IP addresses for tablet servers

Typically admins know their hosts by hostname instead of by IP address.
This rejiggers the code a bit to save the original host/port instead of
the resolved address, so that the output contains a hostname instead
of an IP.

Tested manually against a cluster (hard to add an assertion since we
don't have the ability to mock DNS).

Change-Id: I8164dca050fd1adcc034a91cebc241e6fff8a117
---
M src/kudu/tools/ksck-test.cc
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
4 files changed, 30 insertions(+), 19 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8164dca050fd1adcc034a91cebc241e6fff8a117
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] ksck: multi-thread the fetching of replica info from tablet servers

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: ksck: multi-thread the fetching of replica info from tablet 
servers
..

ksck: multi-thread the fetching of replica info from tablet servers

In clusters with a lot of tablets, fetching the data from each tablet server
can take a while (eg ~100ms per server from my laptop to a test cluster in our
datacenter). For a large cluster (~70 nodes), this makes ksck rather slow.

Multi-threading the fetching makes this much faster (5s vs original 31 seconds
for the above test cluster).

Change-Id: Ib7784697fb227743dccaa98922fb958cd6a3270e
---
M src/kudu/tools/ksck.cc
1 file changed, 22 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib7784697fb227743dccaa98922fb958cd6a3270e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] ksck: report hostnames instead of IP addresses for tablet servers

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: ksck: report hostnames instead of IP addresses for tablet 
servers
..

ksck: report hostnames instead of IP addresses for tablet servers

Typically admins know their hosts by hostname instead of by IP address.
This rejiggers the code a bit to save the original host/port instead of
the resolved address, so that the output contains a hostname instead
of an IP.

Tested manually against a cluster (hard to add an assertion since we
don't have the ability to mock DNS).

Change-Id: I8164dca050fd1adcc034a91cebc241e6fff8a117
---
M src/kudu/tools/ksck.h
M src/kudu/tools/ksck_remote.cc
M src/kudu/tools/ksck_remote.h
3 files changed, 29 insertions(+), 18 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8164dca050fd1adcc034a91cebc241e6fff8a117
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] Remove ASF incubation callouts

[Todd Lipcon (Code Review)]

Hello Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Remove ASF incubation callouts
..

Remove ASF incubation callouts

Now that the resolution has passed, we no longer need to
call out incubation status.

This removes all of the mentions of incubation except for:
- mailing list archive links
- git repository links

These two are currently still at the old addresses, but will be
addressed in a follow-up after infrastructure changes go through
(see INFRA-12312)

Change-Id: Ic71f46177e20a7aa09deccb098841c70eaa5f289
---
D DISCLAIMER
M RELEASING.adoc
M build-support/build_source_release.py
M docs/administration.adoc
M docs/configuration.adoc
M docs/configuration_reference.adoc
M docs/configuration_reference_unsupported.adoc
M docs/contributing.adoc
M docs/developing.adoc
M docs/index.adoc
M docs/installation.adoc
M docs/kudu_impala_integration.adoc
M docs/quickstart.adoc
M docs/release_notes.adoc
M docs/schema_design.adoc
M docs/style_guide.adoc
M docs/transaction_semantics.adoc
M docs/troubleshooting.adoc
M python/README.md
M python/setup.py
20 files changed, 45 insertions(+), 75 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic71f46177e20a7aa09deccb098841c70eaa5f289
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] delete table-test: bump timeout waiting for tablet to start

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: delete_table-test: bump timeout waiting for tablet to start
..


delete_table-test: bump timeout waiting for tablet to start

It seems like TestAutoTombstoneAfterRemoteBootstrapRemoteFails can fail because
it times out waiting for the tablet to restart, particularly in TSAN where
things run slowly. Just bumping the timeout to 60s vs 30s.

Change-Id: I6a3798483f506dd4cee2c685c4eff54c5df6569a
Reviewed-on: http://gerrit.cloudera.org:8080/3689
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/integration-tests/delete_table-test.cc
1 file changed, 1 insertion(+), 1 deletion(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6a3798483f506dd4cee2c685c4eff54c5df6569a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] delete table-test: bump timeout waiting for tablet to start

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: delete_table-test: bump timeout waiting for tablet to start
..


Patch Set 1: Code-Review+2

self-+2ing trivial patch

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6a3798483f506dd4cee2c685c4eff54c5df6569a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] delete table-test: bump timeout waiting for tablet to start

[Todd Lipcon (Code Review)]

Hello Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: delete_table-test: bump timeout waiting for tablet to start
..

delete_table-test: bump timeout waiting for tablet to start

It seems like TestAutoTombstoneAfterRemoteBootstrapRemoteFails can fail because
it times out waiting for the tablet to restart, particularly in TSAN where
things run slowly. Just bumping the timeout to 60s vs 30s.

Change-Id: I6a3798483f506dd4cee2c685c4eff54c5df6569a
---
M src/kudu/integration-tests/delete_table-test.cc
1 file changed, 1 insertion(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6a3798483f506dd4cee2c685c4eff54c5df6569a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] tablet peer: update status message on failure to start

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: tablet_peer: update status message on failure to start
..


tablet_peer: update status message on failure to start

If the tablet peer fails to start up, we were calling SetFailed(),
but this didn't actually update the status message which would
later be reported as part of the TabletStatusPB. This made for
confusing debug experiences. This now surfaces the error.

Change-Id: I6b6e53a33fde296d99be7027dbe75ac057920c20
Reviewed-on: http://gerrit.cloudera.org:8080/3682
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans 
---
M src/kudu/tablet/tablet_peer.cc
M src/kudu/tablet/tablet_peer.h
M src/kudu/tserver/tablet_server-test-base.h
M src/kudu/tserver/tablet_server-test.cc
4 files changed, 22 insertions(+), 8 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6b6e53a33fde296d99be7027dbe75ac057920c20
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] client/sample.cc: fixed a couple of crashes

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: client/sample.cc: fixed a couple of crashes
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3685/1//COMMIT_MSG
Commit Message:

Line 11: while logging some messages from terminating reactor threads.
is there another way we can fix this without forcing callers to uninstall the 
log callback? it seems a little onerous to make users deal with this.


Line 13: Fixed issue with an attempt to access non-existing element
hmm, the bug fix looks reasonable, but I'm curious why we don't see this crash 
always in our precommit test runs?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5fa3b812e6402a113bf5e432a3a451dc4cc3735
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1516 ksck should check for more raft-related status issues (partial)

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1516 ksck should check for more raft-related status issues 
(partial)
..


Patch Set 2:

here's some example output on a cluster with a messed up table:
https://gist.github.com/697f2970c4fbaf5f5888b6864d628968

I think there's some more improvements to be made, like distinguishing between 
an under-replicated-but-available tablet vs an under-replicated-below-majority 
tablet.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Reviewer: Will Berkeley 
Gerrit-HasComments: No

[kudu-CR] C++ client: fix on KuduSession::GetPendingErrors()

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: C++ client: fix on KuduSession::GetPendingErrors()
..


Patch Set 1: Code-Review+2

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

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

[kudu-CR](gh-pages) Clean up community web page and add commits@ mailing list

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Clean up community web page and add commits@ mailing list
..


Patch Set 3: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f11c5548f3f6870b9db497bcd1daf59a238e749
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] [c++-client] add LESS and GREATER column predicates

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [c++-client] add LESS and GREATER column predicates
..


Patch Set 1: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3674/1/src/kudu/common/column_predicate.cc
File src/kudu/common/column_predicate.cc:

PS1, Line 98:  
nit: extra space


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

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

[kudu-CR] KUDU-1516 ksck should check for more raft-related status issues (partial)

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1516 ksck should check for more raft-related status issues 
(partial)
..


Patch Set 1:

BTW, I tried this on a cluster with a bad table:
WARNING: Unable to connect to Tablet Server 5fb7d6c7083943059521e03d6ece2863 
(10.20.132.112:7050) because Network error: Client connection negotiation 
failed: client connection to 10.20.132.112:7050: connect: Connection refused 
(error 111)
WARNING: Unable to connect to Tablet Server acd8306f95334ec1bfce8cb30d7ca36d 
(10.20.126.115:7050) because Network error: Client connection negotiation 
failed: client connection to 10.20.126.115:7050: connect: Connection refused 
(error 111)
WARNING: Unable to connect to Tablet Server dff78a5acdbb4a47ba2c7a62d1bcc5ee 
(10.20.132.107:7050) because Network error: Client connection negotiation 
failed: client connection to 10.20.132.107:7050: connect: Connection refused 
(error 111)
WARNING: Connected to 69 Tablet Servers, 3 weren't reachable
WARNING: Tablet 3bf432551c5d4c529616f8e7ce829424 of table 'usertable' does not 
have a majority of replicas in RUNNING state
WARNING: Tablet 2f652871b74b4d0f9bf99e730486a451 of table 'usertable' does not 
have a majority of replicas in RUNNING state
WARNING: Tablet b009973af71842cf99e10d25254b5557 of table 'usertable' does not 
have a majority of replicas in RUNNING state
WARNING: Tablet 71ca44eebda44903868014175e02862a of table 'usertable' does not 
have a majority of replicas in RUNNING state
WARNING: Table usertable has 4 bad tablets
INFO: Table IntegrationTestBigLinkedListHeads is HEALTHY
INFO: Table IntegrationTestBigLinkedList is HEALTHY
WARNING: 1 out of 3 tables are not in a healthy state
==
Errors:
==
Tablet server aliveness check error: Network error: Not all Tablet Servers are 
reachable
Table consistency check error: Corruption: 1 tables are bad

FAILED

>From this output you can see how it would be useful to give slightly more info 
>on why the bad tablets are bad. Let me know if you'll have time to keep 
>working on this - otherwise I might try to take it from where you left off.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec6590ba52548a9ee11d63269b134320b10809da
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1374: send full tablet report when new leader master is detected

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1374: send full tablet report when new leader master is 
detected
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3643/5//COMMIT_MSG
Commit Message:

Line 13: there was a way to mock a server-side krpc component as easily as it 
is to
what about injecting a master crash in the AsyncSendFoo() method? that would be 
likely to trigger this in many cases, right? (not a guarantee that the response 
was quicker than the starting of the async stuff, but pretty likely I think?)


Line 15: multi-master tests, though.
is there a test you can loop before/after to verify this?


http://gerrit.cloudera.org:8080/#/c/3643/5/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

Line 175:   // Indicates that the thread should set a full tablet report. Set 
when
s/set/send/


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

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

[kudu-CR] KUDU-1358 (part 3): new multi-master stress test

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1358 (part 3): new multi-master stress test
..


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3611/9/src/kudu/integration-tests/master-stress-test.cc
File src/kudu/integration-tests/master-stress-test.cc:

PS9, Line 73: tT
admin?


Line 219: // TODO: Should be fixed with Exactly Once semantics.
would be nice to tie these to JIRAs


Line 223:   num_altered++;
why not get rid of these local variables and just IncrementBy(1) on the global 
atomic?


Line 323:   SleepFor(MonoDelta::FromMilliseconds(100));
would be good to randomize the sleeps, so you might catch some slightly 
different interleavings


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I40b5b78c100a7b427b2f4aac3a54665e82a9618c
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1358 (part 2): heartbeat to every master

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1358 (part 2): heartbeat to every master
..


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3610/9/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

Line 164:   mutable std::atomic_int next_report_seq_;
hrm, this really has to be mutable?


Line 522: TabletReportState state;
maybe use = { seqno }; here? that way you'll get a warning at some point later 
if you added a field?


PS9, Line 528: const 
maybe makes more sense for this to not be 'const' since it changes the sequence 
number (rather than making it mutable). willing to accept this way too though


http://gerrit.cloudera.org:8080/#/c/3610/9/src/kudu/tserver/heartbeater.h
File src/kudu/tserver/heartbeater.h:

Line 57:   // not dirty once the report has been acknowledged by every master.
this makes it sound like it will keep reporting as "dirty" to all masters until 
all masters have acknowledged. Really, dirtiness is tracked separately per 
master, right?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic85ac4193462d21c989dbd7874b451e8eaab8e3e
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Clean up community web page and add commits@ mailing list

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Clean up community web page and add commits@ mailing list
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8f11c5548f3f6870b9db497bcd1daf59a238e749
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add dropdown menu for Community nav button

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add dropdown menu for Community nav button
..


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3665/2/_includes/top_common.html
File _includes/top_common.html:

PS2, Line 74: http://localhost:4000/
woops


http://gerrit.cloudera.org:8080/#/c/3665/2/css/kudu.css
File css/kudu.css:

Line 125: @media (max-width:870px) {
it's sort of weird that the dropdown disappears at a different width than the 
style switches to the 'hamburger' menu. ie if I just have a not-wide screen, I 
don't get the dropdown, but I still have the horizontal menu.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c0f2671e2257a4249b4b26d1cc8513ee0e47362
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] master: do not delete unknown tablets

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: master: do not delete unknown tablets
..


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3645/5//COMMIT_MSG
Commit Message:

Line 7: master: do not delete unknown tablets
I wonder if we now need some tool to "reformat" a tablet server? or to insert a 
dummy "deleted table" or "deleted tablets" entry into the master? i.e what if 
for some reason we do end up with some unknown tablets that are being reported, 
and we want to delete them to reclaim the space. Right now we'd have to send 
manual delete_tablet RPCs to every server which sounds kind of complicated to 
script up.

Another option would be some kind of "automatically resurrect the tablet based 
on reports" or something?

(thinking about the case here where we have taken a backup of a master and roll 
back in time, for example).

(of course feel free to backlog this, just curious if we anticipate a support 
burden here)


Line 9: Quoting from the multi-master design doc:
nit: can you provide a link or source code path here?


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

PS5, Line 1546: INFO)
WARNING might be more appropriate here? or perhaps INFO if it's an incremental 
report, and WARNING if it's a full report? (since the latter indicates a 
persistent condition whereas the former might be a transient issue as in the 
race described above?)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I331f2d5bb06c38daa7b09854dbb24a7881723551
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Add weekly update for 7/18

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add weekly update for 7/18
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0ee48c282377263dace0ac66fe3043aaf856c47
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update the docs webpages to reflect the master branch

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Update the docs webpages to reflect the master branch
..


Patch Set 1:

The reason they're out of date is that we typically wait for a release to 
publish the new docs. If we want to start publishing master ("trunk") docs, I 
think we should make it clear in the URL (and maybe a header on the pages) 
saying something like "This page refers to an unreleased development version of 
Apache Kudu." with some kind of link back to the latest release.

Otherwise the average user (who does not build and run trunk) will be confused 
when they see APIs or features that aren't available in what they're running. 
For example, check out what LLVM does with the big red WARNING: 
http://llvm.org/docs/ReleaseNotes.html

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5a760ec169880954961f2da2a288f4d963e87d84
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Dinesh Bhat 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1358 (part 1): master should accept heartbeat even if follower

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1358 (part 1): master should accept heartbeat even if 
follower
..


Patch Set 9:

(1 comment)

Any way to system test this, like calling ListTabletServers against a follower 
master? (or visiting its /tablet-servers web page?)

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

PS9, Line 366:   };
DISALLOW_COPY_AND_ASSIGN


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I578674927b65b4171e8437de8515130e4a0ed139
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Fix flaky disk reservation-itest

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Fix flaky disk_reservation-itest
..


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ica86390b49d459e079807d777e97c47fa35134d1
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Fix flaky disk reservation-itest

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Fix flaky disk_reservation-itest
..


Fix flaky disk_reservation-itest

There are two fixes in this patch for two separate types of failures
seen on Jenkins for this test:

1. Fix a data race in DiskReservationITest.TestFillMultipleDisks

We can't override gflag strings at runtime in a thread-safe manner,
although this test was attempting to.

Take what used to be a single parsed string gflag and replace it with 2
path strings and 2 integer overrides, one for each path. That makes 4
new test-only gflags total. Only the integer flags are modified at
runtime.

2. Fix a startup race between the TestWorkload client thread and
   SetFlags() in DiskReservationITest.TestWalWriteToFullDiskAborts

We need to wait for some rows to be written after starting up the
TestWorkload threads in TestWalWriteToFullDiskAborts before we allow the
TS to crash by setting gflags. If we don't, the test gets confused
because the TestWorkload client thread may not be able to resolve where
the tablet is located. The previous failures were because we sometimes
managed to crash the TS before it sent its tablet report to the master.

After applying these changes, I looped disk_reservation-itest 1000x in
TSAN mode and got no failures.

Change-Id: Ica86390b49d459e079807d777e97c47fa35134d1
Reviewed-on: http://gerrit.cloudera.org:8080/3652
Tested-by: Mike Percy 
Reviewed-by: Todd Lipcon 
---
M src/kudu/integration-tests/disk_reservation-itest.cc
M src/kudu/util/env_util.cc
2 files changed, 61 insertions(+), 29 deletions(-)

Approvals:
  Mike Percy: Verified
  Todd Lipcon: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ica86390b49d459e079807d777e97c47fa35134d1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](gh-pages) Add weekly update for 7/18

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Add weekly update for 7/18
..

Add weekly update for 7/18

Change-Id: Ib0ee48c282377263dace0ac66fe3043aaf856c47
---
A _posts/2016-07-18-weekly-update.md
1 file changed, 59 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib0ee48c282377263dace0ac66fe3043aaf856c47
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] Memory tracking for result tracker

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Memory tracking for result tracker
..


Patch Set 15:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/3627/15/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 47: struct ScopedMemTrackerUpdater {
would using ScopedCleanup with a lambda be easier? eg:

auto update_memtracker = MakeScopedCleanup([]{
  tracker->Release(...)
});

on second thought, now that I understand it, maybe not... but leaving the 
comment just because MakeScopedCleanup is neat and you might not have seen it 
when it was added :)


Line 70: ResultTracker::ResultTracker(const shared_ptr& mem_tracker)
I think with C++11 it's preferable to take shared_ptr mem_tracker 
(byvalue) and then assign with std::move


http://gerrit.cloudera.org:8080/#/c/3627/15/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

PS15, Line 228: ns'
typo


Line 231: return client_state_map_bytes_;
this needs an ANNOTATE_UNPROTECTED_READ or a lock, right?


http://gerrit.cloudera.org:8080/#/c/3627/15/src/kudu/rpc/rpc-stress-test.cc
File src/kudu/rpc/rpc-stress-test.cc:

PS15, Line 259: Sleep t
is this fragile? why not a loop? also, why is there a delay here? shouldn't it 
be updated already before AddExactlyOnce returns?


Line 262: ASSERT_GT(mem_consumption, original_resp.SpaceUsed());
I think grabbing the original consumption before the RPC, and making sure that 
it _increases_ by more than SpacedUsed would be more robust, since the 
"SpaceUsed" of this protobuf is pretty small, potentially even less than the 
base size of the structure


PS15, Line 283: double 
hm, I'm actually slightly surprised by this. Isn't this pretty dependent on the 
implementation of the map?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3b81dda41c8bc7f70380ce426142c34afe6f1625
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a design doc for rpc retry/failover semantics

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a design doc for rpc retry/failover semantics
..


Patch Set 5:

We should make sure not to forget about this one before calling all the replay 
cache work "done"

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Idc2aa40486153b39724e1c9bd09c626b829274c6
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] rw mutex: prevent recursive use

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: rw_mutex: prevent recursive use
..


Patch Set 6: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db
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] rw mutex: prevent recursive use

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: rw_mutex: prevent recursive use
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3641/1/src/kudu/util/rw_mutex-test.cc
File src/kudu/util/rw_mutex-test.cc:

Line 52: // Multi-threaded test that tries to find deadlocks in the RWMutex 
wrapper.
is this actually a realistic scenario to be concerned about? don't we expect 
pthread to work ok?


Line 71: // Do something that the compiler won't optimize away.
I dont think the compiler can optimize this stuff away because it includes 
library calls to pthread. also the volatile load can't be optimized out


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ae30ec123a16c39ef0c15ee2d2176f807df03db
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rw mutex: add configurable priority

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: rw_mutex: add configurable priority
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I16ba6cd041f126c94e63fa07a1e84c88db6778d7
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Integrate the ResultTracker into the rpc subsystem

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Integrate the ResultTracker into the rpc subsystem
..


Patch Set 24:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/3192/24/src/kudu/rpc/protoc-gen-krpc.cc
File src/kudu/rpc/protoc-gen-krpc.cc:

Line 456: "result_tracker_ = result_tracker;"
this can use an initializer list, right?


http://gerrit.cloudera.org:8080/#/c/3192/24/src/kudu/rpc/rpc-stress-test.cc
File src/kudu/rpc/rpc-stress-test.cc:

PS24, Line 59: We don't use CalculatorServiceRpc 
not sure what class this is referring to. I guess this is coming in a later 
patch?


Line 68:  atomic_int* attempt_nos) {
not following why this is atomic. you're fetch_add()ing to it from the main 
thread in the constructor, and then it's not accessed from the started threads. 
Why not just take an int attempt_num here?


Line 70:   client_sleep_for_ms = client_sleep;
above two lines can be from an initializer list


PS24, Line 83: a the 
typo


Line 91: uint64_t client_sleep_for_ms;
make these above two const


Line 100:   string client_id_;
this is just a constant, right? can you just make it a const char* kClientId?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6
Gerrit-PatchSet: 24
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add information about Exactly Once RPC semantics to rpc.md

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Add information about Exactly Once RPC semantics to rpc.md
..


Add information about Exactly Once RPC semantics to rpc.md

This patch adds some info about exactly one rpc semantics to
rpc.md, mostly from a usage perspective.

Change-Id: I8acf4e830eb673b6a696b12188bb9aafb65b261e
Reviewed-on: http://gerrit.cloudera.org:8080/3503
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M docs/design-docs/rpc.md
1 file changed, 28 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I8acf4e830eb673b6a696b12188bb9aafb65b261e
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ToString() method to Proxy

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a ToString() method to Proxy

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Add a ToString() method to Proxy
..


Add a ToString() method to Proxy

ReplicatedRpc takes the server proxy type as a template argument and
uses its ToString() method to print out details in case of error. Usually
this is RemoteTablet, which has a ToString() method, but that might
not always be the case.

In fact, in a test in a follow up patch ReplicatedRpc takes Proxy as the
server proxy type and compilation would fail due to a missing ToString().
We could make ReplicatedRpc not use the ToString() method, but it seems
very helpful to have it so this patch adds it to Proxy instead.

Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e
Reviewed-on: http://gerrit.cloudera.org:8080/3502
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/rpc/proxy.cc
M src/kudu/rpc/proxy.h
M src/kudu/rpc/rpc-test.cc
3 files changed, 12 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Add a ResultTracker class that will track server side results

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 26: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3190/26/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 244:   // CompletionRecords.
OK, I still think this iteration order thing is fragile, but I've made a note 
to come back to it.


http://gerrit.cloudera.org:8080/#/c/3190/26/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

Line 131: // result_tracker_->TrackRpcOrChangeDriver(request_id);
woohoo, I like this much better. thanks for taking the time to re-work it


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 26
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a ResultTracker class that will track server side results

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Add a ResultTracker class that will track server side results
..


Add a ResultTracker class that will track server side results

This adds the initial version of the ResultTracker class that will be
responsible for tracking responses when we want exactly once call semantics.

While this is minimally working, i.e it's able to track responses and
can be used for exactly once semantics, it's not complete. Future patches
will address the missing functionality.
Still missing are:
- Time based client state cleaning.
- Watermark based per client state cleaning.

I initially had a unit test for this class, but it relied on templating
out the RpcContext (since it's not easy to build one for a unit test
and it's not clear what it would do) which became problematic as the
class grew. So I decided to add a new rpc-stress-tess that will test
this class within the rpc framework. I feel it's easier to do it that
way and that, since the client is not being used, the test can still
be very straight forward. This test is in a followup patch as it can
only run after the integration with the rest of the rpc subsystem.

Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Reviewed-on: http://gerrit.cloudera.org:8080/3190
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon 
---
M src/kudu/rpc/CMakeLists.txt
A src/kudu/rpc/result_tracker.cc
A src/kudu/rpc/result_tracker.h
M src/kudu/rpc/rpc_context.h
4 files changed, 599 insertions(+), 0 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 27
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] master: add read-write lock to serialize operations around elections

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: master: add read-write lock to serialize operations around 
elections
..


Patch Set 6: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3550/6//COMMIT_MSG
Commit Message:

Line 30:   RPCs won't fill up its service queue and can still process 
heartbeats.
ah yes, we had basically this problem with the original HDFS HA implementation!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rw mutex: add configurable priority

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: rw_mutex: add configurable priority
..


Patch Set 3:

I have experienced in the past an issue where the fairness policy causes a 
deadlock. See 
https://issues.apache.org/jira/browse/HDFS-2223?focusedCommentId=13097647=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13097647
 for example.

I think the issue only happens when you recursively acquire the read lock, or 
if you are holding the read lock while waiting on another thread which needs to 
acquire the read lock. I'm not sure if we have cases of either, but we should 
probably document this danger.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I16ba6cd041f126c94e63fa07a1e84c88db6778d7
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] master: reduce timeout for master to tserver rpcs

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: master: reduce timeout for master to tserver rpcs
..


Patch Set 2:

I think we bumped this to be pretty high because we found that creating a new 
tablet can actually take reasonably long (it involves a bunch of fsyncs, 
preallocating log, etc) especially when there are lots of them going on at the 
same time (as when you create a table with 10s of tablets per server).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06b5f32c0295f0a9d24ff42695905858ae6101f6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a ResultTracker class that will track server side results

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 22:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3190/22/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 203:   // from before we reply to all the other ongoing ones.
why are we guaranteed that 'completion_record' that we're copying from is at 
the beginning of the list? if I understand correctly, you're saying we go in 
reverse order so we get to 'completion_record' last, but don't understand why 
that property holds


http://gerrit.cloudera.org:8080/#/c/3190/22/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

PS22, Line 44: client ID and same sequence number
typo: same client ID and sequence number


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6718951a9998a6c9b0db35e8f09ff8304591e8b1
Gerrit-PatchSet: 22
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Make EraseKeyReturnValuePtr in map-util work with smart pointers.

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Make EraseKeyReturnValuePtr in map-util work with smart 
pointers.
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a FindPointeeOrNull method to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a FindPointeeOrNull method to map-util
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a5d26bdd39e8d12382eb460cb75e04b645e3b2d
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 9: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a FindPointeeOrNull method to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a FindPointeeOrNull method to map-util
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a5d26bdd39e8d12382eb460cb75e04b645e3b2d
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3593/8/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:

Line 742: const MvccSnapshot& 
snRollingDiskRowSetWriter* out) {
err?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 8
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a EraseKeyReturnSmartPtrValue to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a EraseKeyReturnSmartPtrValue to map-util
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3595/3/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

Line 707: EraseKeyReturnSmartPtrValue(Collection* const collection,
this is more generic than just smart pointer, right? in fact, this is identical 
to the above function except for doing 'std::move' and relying on the default 
constructor instead of just using 'NULL'.

In other words, I think your new function implements the original function too, 
so if you just changed the original to do what you're doing here, you could use 
it in both cases without adding new code?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I65d17dd26317f77c4c0aa4b8ec00fbe51f8d1ced
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a FindPointeeOrNull method to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a FindPointeeOrNull method to map-util
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3594/3/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

PS3, Line 240: nN
typo


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a5d26bdd39e8d12382eb460cb75e04b645e3b2d
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3593/7/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

PS7, Line 800: CHECK
this seems to have turned back to a CHECK instead of DCHECK


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] rw mutex: add try lock methods

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: rw_mutex: add try lock methods
..


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic3dd4f406b5f90898870083ed3143667d1627bd6
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] rw mutex: add configurable priority

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: rw_mutex: add configurable priority
..


Patch Set 2:

Is there any possibility that the different priorities could create a case 
where OSX would deadlock but Linux wouldn't? am slightly worried about 
introducing different semantics on the different platforms (and not just a perf 
difference)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I16ba6cd041f126c94e63fa07a1e84c88db6778d7
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 4: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3593/4/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

Line 814: const typename MapContainer::value_type::second_type&
I won't be surprised if you need a non-const reference return value instead of 
a const one when you try to use this, but guess we will find out :)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add ComputeIfAbsent methods to map-util

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add ComputeIfAbsent methods to map-util
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3593/3/src/kudu/gutil/map-util.h
File src/kudu/gutil/map-util.h:

PS3, Line 779: it 
exist


PS3, Line 791: std::function 
rather than using std::function I think you can templatize on a function type 
here. http://stackoverflow.com/questions/14677997/stdfunction-vs-template seems 
to agree this is usually preferred (the optimizer isn't always smart enough to 
see the equivalence)


PS3, Line 795: CH
I think a DCHECK would be better here, since we probably want this function to 
be as small as possible to be inlined


Line 806: ComputeIfAbsentReturnAbsense(MapContainer* container,
could you implement ComputeIfAbsent as just calling 
ComputeIfAbsentReturnAbsense?


http://gerrit.cloudera.org:8080/#/c/3593/3/src/kudu/util/map-util-test.cc
File src/kudu/util/map-util-test.cc:

PS3, Line 54:  {
nit: inconsistent spacing (also below)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iba9fa8038e699e66d34ce541cd02c77f46691315
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Integrate the result tracker with writes

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Integrate the result tracker with writes
..


Patch Set 17:

(18 comments)

http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/client/batcher.cc
File src/kudu/client/batcher.cc:

Line 51: #include "kudu/rpc/rpc_header.pb.h"
unused?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

Line 182:   optional rpc.RequestIdPB request_id = 100;
why using such a high id here? seems like it wastes a byte


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 162: completion_record->final_driver = true;
this is a big code smell -- this function should be read-only, not somehow have 
a side effect. If you want it to have a side effect, rename it to 
'BecomeFinalDriver' or something? 'UpgradeToFinalDriver'?

Also, the docs in the header file should reference this concept of being the 
'final driver'


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

Line 226: bool final_driver = false;
doc this

also, weird that this field has a default but the others don't


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:

Line 251:  WriteResponsePB* response);
mentioned on rev 1, but I dont like the side effect here without an 
accompanying method rename and doc update


Line 1243:   if (tracking_results) {
any way to extract this new bit of code to a separate function?


Line 1279:   LOG(FATAL) << "Couldn't change bootstrapping txn to driver 
after 10 attempts for request: "
this seems like a kind of arbitrary thing... should this be a DFATAL and keep 
looping?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/tablet_peer.cc
File src/kudu/tablet/tablet_peer.cc:

Line 130: const scoped_refptr 
result_tracker,
reference


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/transaction_driver.cc
File src/kudu/tablet/transactions/transaction_driver.cc:

Line 212:   if (state()->are_results_tracked()) {
change to if (!...) return


Line 314: case REPLICATING:
some spurious changes here


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

PS17, Line 117: // 1 - When becoming leader, a replica has already prepared all 
the requests that it received
  : // from the previous leader that it will try to 
replicate/commit itself.
  : /
not quite understanding this sentence


PS17, Line 122: Requests originated on other replicas 
not sure quite what you mean here. Do you mean operations which were started 
while the node is a follower? Or "originated" meaning "the first attempt"?


PS17, Line 158: FailAndRespond() 
hrm, is this FailAndRespond here actually responding to any RPCs in the current 
design? Can we encapsulate this "precedence" behavior into something like the 
'UpgradeToExclusiveDriver' call I mentioned in the other review?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tablet/transactions/write_transaction.h
File src/kudu/tablet/transactions/write_transaction.h:

PS17, Line 196: c_
typo


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/remote_bootstrap_session-test.cc
File src/kudu/tserver/remote_bootstrap_session-test.cc:

Line 137:  dummy,
can just use scoped_refptr(), no?


http://gerrit.cloudera.org:8080/#/c/3449/17/src/kudu/tserver/tablet_service.cc
File src/kudu/tserver/tablet_service.cc:

Line 280: const char* ALREADY_PRESENT_ERROR = "There's already an attempt at 
the same operation "
is this used?


Line 295:   //LOG(INFO) << "Responding error to: " << 
context_->request_pb()->DebugString();
remove


Line 764:   // attempted elsewhere.
this probably needs to be moved into tablet_peer


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1fa2f8db33653960f4749237b8993baba0929893
Gerrit-PatchSet: 17
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a fault injection point after the leader sends a request

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a fault injection point _after_ the leader sends a request
..


Patch Set 2:

(1 comment)

why not merge this commit with whatever uses it? not a fan of these commits 
with no accompanying coverage/usage

http://gerrit.cloudera.org:8080/#/c/3568/2/src/kudu/consensus/consensus_peers.cc
File src/kudu/consensus/consensus_peers.cc:

Line 57: DEFINE_double(fault_crash_after_leader_request_fraction, 0.0,
I think it's clearer to say "on leader response" rather than "after leader 
request"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I504d74ac3ab3a8cb120d4e5ecee308d9846f3829
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a test for the integration of RequestTracker with the client and ResultTracker with the server

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a test for the integration of RequestTracker with the 
client and ResultTracker with the server
..


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3505/9/src/kudu/rpc/rpc-test-base.h
File src/kudu/rpc/rpc-test-base.h:

Line 273: context->RespondSuccess();
would be good for this to randomly fail sometimes to test that code path


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a test for the integration of RequestTracker with the client and ResultTracker with the server

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a test for the integration of RequestTracker with the 
client and ResultTracker with the server
..


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3505/9//COMMIT_MSG
Commit Message:

Line 7: Add a test for the integration of RequestTracker with the client and 
ResultTracker with the server
hrm, this commit message seems inaccurate - this is an RPC-system-only test, 
right?


http://gerrit.cloudera.org:8080/#/c/3505/9/src/kudu/rpc/rpc-stress-test.cc
File src/kudu/rpc/rpc-stress-test.cc:

PS9, Line 143: very short timeout
I dont see the very short timeout


Line 172:   rpc_->SendRpc();
where's the sleep? need to update some comments?


PS9, Line 207: random amount
not random?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add information about Exactly Once RPC semantics to rpc.md

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add information about Exactly Once RPC semantics to rpc.md
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3503/3/docs/design-docs/rpc.md
File docs/design-docs/rpc.md:

Line 192: care is taken to persist results (make responses live through 
restarts),
see comments on earlier rev about some of the wording


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8acf4e830eb673b6a696b12188bb9aafb65b261e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add information about Exactly Once RPC semantics to rpc.md

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add information about Exactly Once RPC semantics to rpc.md
..


Patch Set 3:

also, perhaps this should be merged into the 'integrate into the rpc subsystem' 
patch?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8acf4e830eb673b6a696b12188bb9aafb65b261e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a RpcContext::RespondFailure() method

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a RpcContext::RespondFailure() method
..


Patch Set 15:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3191/15/src/kudu/rpc/rpc_context.h
File src/kudu/rpc/rpc_context.h:

PS15, Line 88: uce
typo


PS15, Line 89: cl
typo


Line 91:   void RespondFailure();
I still think this would be less confusing if it were called 'RespondNoCache' 
to avoid having a _fourth_ confusing option for errors


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9f1387ba0f837046a8056e77f73a3982f06c73a2
Gerrit-PatchSet: 15
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Add a ToString() method to Proxy

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 3:

see note above about adding an assertion

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Add a ResultTracker class that will track server side results

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a ResultTracker class that will track server side results
..


Patch Set 17:

(29 comments)

http://gerrit.cloudera.org:8080/#/c/3190/17/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 54:   }
what do you think about adding a utility function to map-util like 
https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ConcurrentHashMap.html#computeIfAbsent-K-java.util.function.Function-
 ?

Then the above 10 lines or so could be:

  ClientState* client_state = ComputeIfAbsent(_, request_id.client_id(),
   []{ return unique_ptr(new ClientState()); }).get();


Line 76: CHECK(client_state->completion_records.emplace(request_id.seq_no(),
I think you can just emplace(request_id.seq_no(), completion_record) here 
without constructing a unique_ptr and std::moving it, since unique_ptr has a 
unique_ptr(T* val) constructor

(same is true above)


PS17, Line 85: means a tablet is being bootstrapped for the second time,
since this code is in the RPC system I think it would be better to be more 
general in the description here. Plus I think you decided that this isn't the 
only case in which this happens, right?


Line 123:   CompletionRecord* completion_record = cr_it->second.get();
another potential for a map-util function here


Line 153:   // ... if we did find a CompletionRecord return true if we're the 
driver of false
s/of/or/


Line 158: void ResultTracker::LogAndTraceAndRespondSuccess(RpcContext* context,
these methods are duplicating a lot of RpcContext. Remind me again why we can't 
just call context->RespondSuccess()? (perhaps after breaking it out into one 
that takes a PB instead of using 'response_pb_'


PS17, Line 191: e(
nit: add 'Unlocked' to the name


PS17, Line 194: PREDICT_TRUE
no need for PREDICT inside CHECK (it's already implicitly part of CHECK)


Line 206: const Message* response) {
should this be a reference? it doesn't look like it hangs onto the pointer


Line 219:   CHECK(completion_record->driver_attempt_no == 
request_id.attempt_no());
CHECK_EQ


Line 235:   completion_record->ongoing_rpcs.erase(orpc_iter.base()));
hrm, not really familiar with what this is doing. it seems odd though that you 
increment it and _then_ erase it...


http://gerrit.cloudera.org:8080/#/c/3190/17/src/kudu/rpc/result_tracker.h
File src/kudu/rpc/result_tracker.h:

Line 19: #include 
can you get away with a forward decl? am guessing to do so you'll have to 
define ctor and ctor in the .cc file instead of implicit ones, but that's 
probably a good idea anyway


Line 28: #include "kudu/gutil/stl_util.h"
nit: sort


Line 35: // A ResultTracker for RPC results.
somewhere in here I think it would be worth a sentence or two saying that in 
most cases this is internal to the RPC system (ie handlers don't call the 
Respond* methods directly)


PS17, Line 38: sequence number
sequence number and client ID


PS17, Line 41: rpc
nit: s/rpc/RPC/ here and elsewhere


PS17, Line 44: id 
nit: capitalize ID here and elsewhere


PS17, Line 61: being completed,
being handled


PS17, Line 70: od 
methods


PS17, Line 81: be the only handler,
this isn't necessarily true -- the client could have gotten a timeout and 
decided to retry while the original attempt was still running, right?


PS17, Line 84: hanlder 
typo


Line 85: // previous leader originating update.
I think this description is missing one key thing that distinguishes this case 
from the "two retries on the same server" case. Here, we actually know a priori 
that the original leader-originated operation is the one that is going to 
succeed (it has to, because it has been committed), and therefore it needs to 
"steal" the handler role, even if it arrives after a client-originated request. 
However, in the case that you have a non-replicated operation (like two 
client-originated requests) you are still free to arbitrarily assign which one 
gets the ownership and let the other one tack on.

So this whole section is really about "stealing ownership" rather than 
"multiple handlers", right?


PS17, Line 89:  must be handled 
not sure what this means.. do you mean that the responses must be mutated 
exclusively by one handler?


PS17, Line 94: there o
missing a word


Line 182:   void RecordCompletionAndRespond(const RequestIdPB& request_id,
This and the three methods below are only called via RpcContext, right? I think 
it's worth noting for each method whether it's "user-facing" or "rpc-system 
internal"


Line 221: int64_t driver_attempt_no;
what about if it's completed? this is the attempt number that successfully 
handled it?


Line 227:   struct ClientState {
doc


PS17, Line 242: it's
nit: its

also, I dont know what "its own" means, exactly


Line 243:   // 2 - It's the driver of the RPC and the RPC has no handler (was 
attached).
what is "was attached"?


-- 
To view, visit 

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..


Patch Set 3:

did you see my note about a test?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] master: add read-write lock to serialize operations around elections

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: master: add read-write lock to serialize operations around 
elections
..


Patch Set 3:

(8 comments)

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

PS3, Line 345:  l.c
could be more descriptive here?


PS3, Line 367: aborting the current task...
hmm, this (old) log message is nonsense, right?


Line 657: e.second->WaitTasksCompletion();
does this put a restriction on the tasks themselves that they may not block on 
the leader lock? otherwise is there a deadlock case?


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

PS3, Line 304: General status 
a specific example of what types of "bad status" this might represent would be 
useful


PS3, Line 305: leadership_status
you mean leader_status()?
should we DCHECK(catalog_status.ok()) within leader_status() in this case?


PS3, Line 308: Leadership status
would a simple bool suffice?


PS3, Line 316: '.
and returns false


PS3, Line 691: etc
what about read operations? should you specifically say write operations? are 
read operations already protected by the tableinfo locks, etc? or should we 
acquire in them too to prevent a race where the Visit*() functions clear the 
maps just as a read request arrives (but after the read request checks 
leadership)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5084c09f1a77ccf620fb6cd621094c4778d636f8
Gerrit-PatchSet: 3
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](gh-pages) Blog post for 0.9.1 announcement

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Blog post for 0.9.1 announcement
..

Blog post for 0.9.1 announcement

Change-Id: Ibff6bed45702b2b64c0ab977322180b785c34886
---
A _posts/2016-07-01-apache-kudu-0-9-1-released.md
1 file changed, 13 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibff6bed45702b2b64c0ab977322180b785c34886
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] KUDU-763 consensus queue metrics on followers are messed up

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-763 consensus queue metrics on followers are messed up
..


Patch Set 2:

(2 comments)

Should have a test as well for this

http://gerrit.cloudera.org:8080/#/c/3501/2/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

Line 77:   "not all peers. Not meaningful for 
non-leader tablet replicas.");
I do think we should actually make the data at least be "0" in the case that 
it's a follower. Right now, the numbers go negative which is pretty odd. 
Remember that the metrics aren't just exposed on that web UI, but also in other 
systems which capture metrics.


http://gerrit.cloudera.org:8080/#/c/3501/2/src/kudu/tserver/tserver-path-handlers.cc
File src/kudu/tserver/tserver-path-handlers.cc:

Line 251: peer.has_last_known_addr() ? Substitute("$0:$1",
would be good to move these web ui improvements to a separate patch


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9fb0d45f85786b9e2631b5dc0bf044a9d3192a39
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] deltafile-test: fix TestEmptyFileIsAborted for file block manager

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: deltafile-test: fix TestEmptyFileIsAborted for file block 
manager
..


Patch Set 1: Code-Review+2

Did you verify this fixes with FBM enabled?

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

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

[kudu-CR] Try again to disable core dumps in some tests

[Todd Lipcon (Code Review)]

Hello Mike Percy,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Try again to disable core dumps in some tests
..

Try again to disable core dumps in some tests

133767185047987e1d7f768d09db950dc8e38897 attempted to disable core
dumps in some tests by adding a --disable_core_dumps flag. However,
I had put the handling of this flag in the InitKuduOrDie() function,
which actually is called prior to flags being parsed, so the flag
didn't work as intended.

I noticed that the fix was ineffectual because the tests that were
supposed to be de-flaked by it remained flaky. After setting up
a VM which matched the test environment I noticed that they were
still flaky due to core dumps, and figured out this issue.

The fix is just to move the DisableCoreDumps() call to the spot
where we parse flags.

Change-Id: I51f3d5d8c72622b8a3b8b4a13dd343e864a78db3
---
M src/kudu/util/flags.cc
M src/kudu/util/init.cc
2 files changed, 11 insertions(+), 11 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I51f3d5d8c72622b8a3b8b4a13dd343e864a78db3
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Mike Percy 

[kudu-CR] [java-client] refactor AsyncKuduSession

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: [java-client] refactor AsyncKuduSession
..


Patch Set 7:

Should we change the default buffer size up by an order of magnitude so that 
users don't experience this as a regression? We should also release note the 
changed client behavior.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ee6d029b1a56e254bfb9a870917883abeadb6b8
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update docs for 0.9.1

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

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

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

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

Change subject: Update docs for 0.9.1
..

Update docs for 0.9.1

Change-Id: Ida85e4a7b583c6d66e961c3435a6f0bcc363eacc
---
M apidocs/allclasses-frame.html
M apidocs/allclasses-noframe.html
M apidocs/constant-values.html
M apidocs/deprecated-list.html
M apidocs/help-doc.html
M apidocs/index-all.html
M apidocs/index.html
M apidocs/org/kududb/ColumnSchema.html
M apidocs/org/kududb/Schema.html
M apidocs/org/kududb/Type.html
M apidocs/org/kududb/annotations/InterfaceAudience.html
M apidocs/org/kududb/annotations/InterfaceStability.html
M apidocs/org/kududb/annotations/class-use/InterfaceAudience.html
M apidocs/org/kududb/annotations/class-use/InterfaceStability.html
M apidocs/org/kududb/annotations/package-frame.html
M apidocs/org/kududb/annotations/package-summary.html
M apidocs/org/kududb/annotations/package-tree.html
M apidocs/org/kududb/annotations/package-use.html
M apidocs/org/kududb/class-use/ColumnSchema.html
M apidocs/org/kududb/class-use/Schema.html
M apidocs/org/kududb/class-use/Type.html
M apidocs/org/kududb/client/AbstractKuduScannerBuilder.html
M apidocs/org/kududb/client/AlterTableOptions.html
M apidocs/org/kududb/client/AlterTableResponse.html
M apidocs/org/kududb/client/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/kududb/client/AsyncKuduClient.html
M apidocs/org/kududb/client/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/kududb/client/AsyncKuduScanner.ReadMode.html
M apidocs/org/kududb/client/AsyncKuduScanner.html
M apidocs/org/kududb/client/AsyncKuduSession.html
M apidocs/org/kududb/client/ColumnRangePredicate.html
M apidocs/org/kududb/client/ConnectionResetException.html
M apidocs/org/kududb/client/CreateTableOptions.html
M apidocs/org/kududb/client/Delete.html
M apidocs/org/kududb/client/DeleteTableResponse.html
M apidocs/org/kududb/client/ExternalConsistencyMode.html
M apidocs/org/kududb/client/HasFailedRpcException.html
M apidocs/org/kududb/client/Insert.html
M apidocs/org/kududb/client/InvalidResponseException.html
M apidocs/org/kududb/client/IsAlterTableDoneResponse.html
M apidocs/org/kududb/client/KuduClient.KuduClientBuilder.html
M apidocs/org/kududb/client/KuduClient.html
M apidocs/org/kududb/client/KuduException.html
M apidocs/org/kududb/client/KuduPredicate.ComparisonOp.html
M apidocs/org/kududb/client/KuduPredicate.html
M apidocs/org/kududb/client/KuduScanToken.KuduScanTokenBuilder.html
M apidocs/org/kududb/client/KuduScanToken.html
M apidocs/org/kududb/client/KuduScanner.KuduScannerBuilder.html
M apidocs/org/kududb/client/KuduScanner.html
M apidocs/org/kududb/client/KuduServerException.html
M apidocs/org/kududb/client/KuduSession.html
M apidocs/org/kududb/client/KuduTable.html
M apidocs/org/kududb/client/ListTablesResponse.html
M apidocs/org/kududb/client/ListTabletServersResponse.html
M apidocs/org/kududb/client/LocatedTablet.Replica.html
M apidocs/org/kududb/client/LocatedTablet.html
M apidocs/org/kududb/client/MasterErrorException.html
M apidocs/org/kududb/client/NoLeaderMasterFoundException.html
M apidocs/org/kududb/client/NonRecoverableException.html
M apidocs/org/kududb/client/Operation.html
M apidocs/org/kududb/client/OperationResponse.html
M apidocs/org/kududb/client/PartialRow.html
M apidocs/org/kududb/client/PleaseThrottleException.html
M apidocs/org/kududb/client/RecoverableException.html
M apidocs/org/kududb/client/RowError.html
M apidocs/org/kududb/client/RowErrorsAndOverflowStatus.html
M apidocs/org/kududb/client/RowResult.html
M apidocs/org/kududb/client/RowResultIterator.html
M apidocs/org/kududb/client/SessionConfiguration.FlushMode.html
M apidocs/org/kududb/client/SessionConfiguration.html
M apidocs/org/kududb/client/Statistics.Statistic.html
M apidocs/org/kududb/client/Statistics.html
M apidocs/org/kududb/client/Status.html
M apidocs/org/kududb/client/TabletServerErrorException.html
M apidocs/org/kududb/client/Update.html
M apidocs/org/kududb/client/Upsert.html
M apidocs/org/kududb/client/class-use/AbstractKuduScannerBuilder.html
M apidocs/org/kududb/client/class-use/AlterTableOptions.html
M apidocs/org/kududb/client/class-use/AlterTableResponse.html
M 
apidocs/org/kududb/client/class-use/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/kududb/client/class-use/AsyncKuduClient.html
M 
apidocs/org/kududb/client/class-use/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/kududb/client/class-use/AsyncKuduScanner.ReadMode.html
M apidocs/org/kududb/client/class-use/AsyncKuduScanner.html
M apidocs/org/kududb/client/class-use/AsyncKuduSession.html
M apidocs/org/kududb/client/class-use/ColumnRangePredicate.html
M apidocs/org/kududb/client/class-use/ConnectionResetException.html
M apidocs/org/kududb/client/class-use/CreateTableOptions.html
M apidocs/org/kududb/client/class-use/Delete.html
M 

[kudu-CR](gh-pages) Update docs for 0.9.1

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Update docs for 0.9.1
..

Update docs for 0.9.1

Change-Id: Ida85e4a7b583c6d66e961c3435a6f0bcc363eacc
---
M apidocs/allclasses-frame.html
M apidocs/allclasses-noframe.html
M apidocs/constant-values.html
M apidocs/deprecated-list.html
M apidocs/help-doc.html
M apidocs/index-all.html
M apidocs/index.html
M apidocs/org/kududb/ColumnSchema.html
M apidocs/org/kududb/Schema.html
M apidocs/org/kududb/Type.html
M apidocs/org/kududb/annotations/InterfaceAudience.html
M apidocs/org/kududb/annotations/InterfaceStability.html
M apidocs/org/kududb/annotations/class-use/InterfaceAudience.html
M apidocs/org/kududb/annotations/class-use/InterfaceStability.html
M apidocs/org/kududb/annotations/package-frame.html
M apidocs/org/kududb/annotations/package-summary.html
M apidocs/org/kududb/annotations/package-tree.html
M apidocs/org/kududb/annotations/package-use.html
M apidocs/org/kududb/class-use/ColumnSchema.html
M apidocs/org/kududb/class-use/Schema.html
M apidocs/org/kududb/class-use/Type.html
M apidocs/org/kududb/client/AbstractKuduScannerBuilder.html
M apidocs/org/kududb/client/AlterTableOptions.html
M apidocs/org/kududb/client/AlterTableResponse.html
M apidocs/org/kududb/client/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/kududb/client/AsyncKuduClient.html
M apidocs/org/kududb/client/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/kududb/client/AsyncKuduScanner.ReadMode.html
M apidocs/org/kududb/client/AsyncKuduScanner.html
M apidocs/org/kududb/client/AsyncKuduSession.html
M apidocs/org/kududb/client/ColumnRangePredicate.html
M apidocs/org/kududb/client/ConnectionResetException.html
M apidocs/org/kududb/client/CreateTableOptions.html
M apidocs/org/kududb/client/Delete.html
M apidocs/org/kududb/client/DeleteTableResponse.html
M apidocs/org/kududb/client/ExternalConsistencyMode.html
M apidocs/org/kududb/client/HasFailedRpcException.html
M apidocs/org/kududb/client/Insert.html
M apidocs/org/kududb/client/InvalidResponseException.html
M apidocs/org/kududb/client/IsAlterTableDoneResponse.html
M apidocs/org/kududb/client/KuduClient.KuduClientBuilder.html
M apidocs/org/kududb/client/KuduClient.html
M apidocs/org/kududb/client/KuduException.html
M apidocs/org/kududb/client/KuduPredicate.ComparisonOp.html
M apidocs/org/kududb/client/KuduPredicate.html
M apidocs/org/kududb/client/KuduScanToken.KuduScanTokenBuilder.html
M apidocs/org/kududb/client/KuduScanToken.html
M apidocs/org/kududb/client/KuduScanner.KuduScannerBuilder.html
M apidocs/org/kududb/client/KuduScanner.html
M apidocs/org/kududb/client/KuduServerException.html
M apidocs/org/kududb/client/KuduSession.html
M apidocs/org/kududb/client/KuduTable.html
M apidocs/org/kududb/client/ListTablesResponse.html
M apidocs/org/kududb/client/ListTabletServersResponse.html
M apidocs/org/kududb/client/LocatedTablet.Replica.html
M apidocs/org/kududb/client/LocatedTablet.html
M apidocs/org/kududb/client/MasterErrorException.html
M apidocs/org/kududb/client/NoLeaderMasterFoundException.html
M apidocs/org/kududb/client/NonRecoverableException.html
M apidocs/org/kududb/client/Operation.html
M apidocs/org/kududb/client/OperationResponse.html
M apidocs/org/kududb/client/PartialRow.html
M apidocs/org/kududb/client/PleaseThrottleException.html
M apidocs/org/kududb/client/RecoverableException.html
M apidocs/org/kududb/client/RowError.html
M apidocs/org/kududb/client/RowErrorsAndOverflowStatus.html
M apidocs/org/kududb/client/RowResult.html
M apidocs/org/kududb/client/RowResultIterator.html
M apidocs/org/kududb/client/SessionConfiguration.FlushMode.html
M apidocs/org/kududb/client/SessionConfiguration.html
M apidocs/org/kududb/client/Statistics.Statistic.html
M apidocs/org/kududb/client/Statistics.html
M apidocs/org/kududb/client/Status.html
M apidocs/org/kududb/client/TabletServerErrorException.html
M apidocs/org/kududb/client/Update.html
M apidocs/org/kududb/client/Upsert.html
M apidocs/org/kududb/client/class-use/AbstractKuduScannerBuilder.html
M apidocs/org/kududb/client/class-use/AlterTableOptions.html
M apidocs/org/kududb/client/class-use/AlterTableResponse.html
M 
apidocs/org/kududb/client/class-use/AsyncKuduClient.AsyncKuduClientBuilder.html
M apidocs/org/kududb/client/class-use/AsyncKuduClient.html
M 
apidocs/org/kududb/client/class-use/AsyncKuduScanner.AsyncKuduScannerBuilder.html
M apidocs/org/kududb/client/class-use/AsyncKuduScanner.ReadMode.html
M apidocs/org/kududb/client/class-use/AsyncKuduScanner.html
M apidocs/org/kududb/client/class-use/AsyncKuduSession.html
M apidocs/org/kududb/client/class-use/ColumnRangePredicate.html
M apidocs/org/kududb/client/class-use/ConnectionResetException.html
M apidocs/org/kududb/client/class-use/CreateTableOptions.html
M apidocs/org/kududb/client/class-use/Delete.html
M 

[kudu-CR] WIP: Integration test for replay cache

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: WIP: Integration test for replay cache
..


Patch Set 5:

Been looking at the new test and the code for the last couple hours.

A couple concerns I have from running the test:
- at least on my system, it doesn't trigger any leader elections. Perhaps we 
need a thread which is randomly cycling through the tservers and stopping them? 
Or explicitly call StartElection to force term bumps and re-elections?
- I ran it through coverage, and perhaps due to the above, there are some lines 
in ResultTracker that aren't covered. For example, in 
RecordCompletionAndRespond:

24820:  225:if (MustHandleRpc(handler_attempt_no, completion_record, 
ongoing_rpc)) {
12410:  226:  if (ongoing_rpc.context != nullptr) {
11873:  227:if (PREDICT_FALSE(ongoing_rpc.response != response)) {
 3746:  228:  
ongoing_rpc.response->CopyFrom(*completion_record->response);
 1873:  229:}
11873:  230:LogAndTraceAndRespondSuccess(ongoing_rpc.context, 
*ongoing_rpc.response);
11873:  231:  }
24820:  232:  orpc_iter = 
completion_record->ongoing_rpcs.erase(orpc_iter);
12410:  233:} else {
#:  234:  ++orpc_iter;
-:  235:}
-:  236:  }
3:  237:}


( indicates a non-covered line). Not sure yet whether this line is actually 
impossible to hit, or just not hit due to a test deficiency.

Will keep reading through this trying to fully understand what's going on. To 
aid that, one suggestion is to do another pass over the ResultTracker API and 
explain some more of the invariants. For example: if one thread uses 
ChangeDriver, what requirements does that impose on the previous driver? (I 
think it means that the previous driver must not call RecordSuccess, and it's 
up to the caller to ensure that the previous driver can't succeed if a new 
driver steals it?)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35722eb1c83f97e886cfe9d6b03ed95bcd62429f
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Update docs on how to run gcovr

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Update docs on how to run gcovr
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3508/1/README.adoc
File README.adoc:

PS1, Line 221: gcc (not clang)
this part is also incorrect (my fault for missing it originally)


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

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

[kudu-CR] Remove unused RunShellProcess() function

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Remove unused RunShellProcess() function
..


Remove unused RunShellProcess() function

Change-Id: I881410660a360d2c6f75046dc95d61453b562bd2
Reviewed-on: http://gerrit.cloudera.org:8080/3533
Reviewed-by: Mike Percy 
Tested-by: Kudu Jenkins
---
M src/kudu/util/os-util.cc
M src/kudu/util/os-util.h
2 files changed, 0 insertions(+), 33 deletions(-)

Approvals:
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I881410660a360d2c6f75046dc95d61453b562bd2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Remove unused RunShellProcess() function

[Todd Lipcon (Code Review)]

Hello Mike Percy,

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

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

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

Change subject: Remove unused RunShellProcess() function
..

Remove unused RunShellProcess() function

Change-Id: I881410660a360d2c6f75046dc95d61453b562bd2
---
M src/kudu/util/os-util.cc
M src/kudu/util/os-util.h
2 files changed, 0 insertions(+), 33 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I881410660a360d2c6f75046dc95d61453b562bd2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 

[kudu-CR] locks: change kudu::shared lock constructor to pass by ref

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: locks: change kudu::shared_lock constructor to pass by ref
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e25a4f519f5cc09f08f8aeda5284eabdadd0c46
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] locks: add new read-write mutex

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: locks: add new read-write mutex
..


Patch Set 5: Code-Review+2

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

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

[kudu-CR](gh-pages) Update links to kudu.apache.org

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Update links to kudu.apache.org
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0523a4840196c92c739aeae5c0de6d52cb1a72a8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update version numbers on releases page to say incubating

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Update version numbers on releases page to say incubating
..


Patch Set 1: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id62cc81f5045d73df6164fae950efd64fa4b5758
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Update links to kudu.apache.org

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Update links to kudu.apache.org
..


Update links to kudu.apache.org

Change-Id: I0523a4840196c92c739aeae5c0de6d52cb1a72a8
Reviewed-on: http://gerrit.cloudera.org:8080/3522
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Todd Lipcon 
---
M README.adoc
M _posts/2016-02-26-apache-kudu-0-7-0-released.md
M _posts/2016-03-10-apache-kudu-0-7-1-released.md
M _posts/2016-04-11-apache-kudu-0-8-0-released.md
M _posts/2016-04-11-weekly-update.md
M _posts/2016-04-18-weekly-update.md
M _posts/2016-04-19-kudu-0-8-0-predicate-improvements.md
M _posts/2016-04-25-weekly-update.md
M _posts/2016-05-03-weekly-update.md
M _posts/2016-06-01-weekly-update.md
M _posts/2016-06-02-no-default-partitioning.md
M _posts/2016-06-10-apache-kudu-0-9-0-released.md
M _posts/2016-06-13-weekly-update.md
M _posts/2016-06-21-weekly-update.md
M docs/release_notes.html
M faq.md
M releases/0.5.0/docs/release_notes.html
M releases/0.6.0/docs/release_notes.html
M releases/0.7.0/docs/release_notes.html
M releases/0.7.1/docs/release_notes.html
M releases/0.8.0/docs/release_notes.html
M releases/0.9.0/docs/release_notes.html
22 files changed, 34 insertions(+), 34 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Todd Lipcon: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0523a4840196c92c739aeae5c0de6d52cb1a72a8
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] tablet: change default bloom filter FP rate to 0.01%

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: tablet: change default bloom filter FP rate to 0.01%
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3517/2//COMMIT_MSG
Commit Message:

PS2, Line 25: 
https://gist.github.com/toddlipcon/1ab9b36b7fbae10b635d3a905e1fe55a
> interesting how the changed graph seems to have a second life towards the e
yea, guess I should have mentioned that -- that was me hacking out block cache 
memory tracking (KUDU-1502)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I99bdd6298349a5be5f1fc3a666fe04305699e293
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR](gh-pages) Update version numbers on releases page to say incubating

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Update version numbers on releases page to say incubating
..

Update version numbers on releases page to say incubating

Change-Id: Id62cc81f5045d73df6164fae950efd64fa4b5758
---
M releases/index.md
1 file changed, 4 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id62cc81f5045d73df6164fae950efd64fa4b5758
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR](gh-pages) Update links to kudu.apache.org

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Update links to kudu.apache.org
..

Update links to kudu.apache.org

Change-Id: I0523a4840196c92c739aeae5c0de6d52cb1a72a8
---
M README.adoc
M _posts/2016-02-26-apache-kudu-0-7-0-released.md
M _posts/2016-03-10-apache-kudu-0-7-1-released.md
M _posts/2016-04-11-apache-kudu-0-8-0-released.md
M _posts/2016-04-11-weekly-update.md
M _posts/2016-04-18-weekly-update.md
M _posts/2016-04-19-kudu-0-8-0-predicate-improvements.md
M _posts/2016-04-25-weekly-update.md
M _posts/2016-05-03-weekly-update.md
M _posts/2016-06-01-weekly-update.md
M _posts/2016-06-02-no-default-partitioning.md
M _posts/2016-06-10-apache-kudu-0-9-0-released.md
M _posts/2016-06-13-weekly-update.md
M _posts/2016-06-21-weekly-update.md
M docs/release_notes.html
M faq.md
M releases/0.5.0/docs/release_notes.html
M releases/0.6.0/docs/release_notes.html
M releases/0.7.0/docs/release_notes.html
M releases/0.7.1/docs/release_notes.html
M releases/0.8.0/docs/release_notes.html
M releases/0.9.0/docs/release_notes.html
22 files changed, 34 insertions(+), 34 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I0523a4840196c92c739aeae5c0de6d52cb1a72a8
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] tablet: change default bloom filter FP rate to 0.01%

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans, Kudu Jenkins,

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

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

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

Change subject: tablet: change default bloom filter FP rate to 0.01%
..

tablet: change default bloom filter FP rate to 0.01%

The old default, 1%, was high enough that in a uniform random write workload,
we ended up needing to read in most of the key blocks even with bloom filters
enabled. On a 5 node cluster, after inserting a few billion rows, the write
throughput dropped dramatically as every batch of writes was seeking and
reading keys off disk.

In testing on the same cluster, changing the FP rate to 0.01% improved the
throughput dramatically (>2x) by reducing the random reads coming off disk. The
cost is a 2x increase in bloom filter size (20 bits per key vs 10) but
20 bits is still a small percentage compared to typical row key sizes
in target applications.

Of course if an application has no random write characteristics and really
cares about disk space, this can always be flipped back.

Screenshots of the inserts/second graph (1hr rolling average) for these tests
are at: https://gist.github.com/toddlipcon/1ab9b36b7fbae10b635d3a905e1fe55a

Change-Id: I99bdd6298349a5be5f1fc3a666fe04305699e293
---
M docs/release_notes.adoc
M src/kudu/tablet/tablet.cc
2 files changed, 10 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I99bdd6298349a5be5f1fc3a666fe04305699e293
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] tablet: change default bloom filter FP rate to 0.01%

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: tablet: change default bloom filter FP rate to 0.01%
..

tablet: change default bloom filter FP rate to 0.01%

The old default, 1%, was high enough that in a uniform random write workload,
we ended up needing to read in most of the key blocks even with bloom filters
enabled. On a 5 node cluster, after inserting a few billion rows, the write
throughput dropped dramatically as every batch of writes was seeking and
reading keys off disk.

In testing on the same cluster, changing the FP rate to 0.01% improved the
throughput dramatically by reducing the random reads coming off disk. The
cost is a 2x increase in bloom filter size (20 bits per key vs 10) but
20 bits is still a small percentage compared to typical row key sizes
in target applications.

Of course if an application has no random write characteristics and really
cares about disk space, this can always be flipped back.

Change-Id: I99bdd6298349a5be5f1fc3a666fe04305699e293
---
M docs/release_notes.adoc
M src/kudu/tablet/tablet.cc
2 files changed, 10 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I99bdd6298349a5be5f1fc3a666fe04305699e293
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] Add a ToString() method to Proxy

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add a ToString() method to Proxy
..


Patch Set 1:

would be nice to add a new assertion to one of the RPC layer tests which calls 
this

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia1e158db09e6e3c188b2725424681187a4b8c72e
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add 6/27 weekly update

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Add 6/27 weekly update
..


Patch Set 2: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ce4bf6e02a5481bf3e032d3345d7afc93c244a2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR](gh-pages) Add 6/27 weekly update

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged.

Change subject: Add 6/27 weekly update
..


Add 6/27 weekly update

Change-Id: I6ce4bf6e02a5481bf3e032d3345d7afc93c244a2
Reviewed-on: http://gerrit.cloudera.org:8080/3515
Reviewed-by: Jean-Daniel Cryans 
Tested-by: Todd Lipcon 
---
A _posts/2016-06-27-weekly-update.md
1 file changed, 91 insertions(+), 0 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Todd Lipcon: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I6ce4bf6e02a5481bf3e032d3345d7afc93c244a2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR](gh-pages) Add 6/27 weekly update

[Todd Lipcon (Code Review)]

Hello Jean-Daniel Cryans,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Add 6/27 weekly update
..

Add 6/27 weekly update

Change-Id: I6ce4bf6e02a5481bf3e032d3345d7afc93c244a2
---
A _posts/2016-06-27-weekly-update.md
1 file changed, 91 insertions(+), 0 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6ce4bf6e02a5481bf3e032d3345d7afc93c244a2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 

[kudu-CR] Implement kudu::optional replacement for boost::optional

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Implement kudu::optional replacement for boost::optional
..


Patch Set 1:

yea, ASAN does insert poisoning between variables on the stack: 
http://llvm.org/docs/doxygen/html/ASanStackFrameLayout_8cpp_source.html

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

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