Adar Dembo has posted comments on this change.

Change subject: [java client] Support add/remove partition

Patch Set 2:


Line 118:    * other or any existing range partitions (unless the existing 
range partitions are dropped as
> But it is transactional in the sense that either all operations are applied
OK, fair. Truth be told, the wait() functionality that's built into the 
KuduTableAlterer (C++ client, but I imagine there's an equivalent in Java too) 
can guarantee that when the "transaction" finishes, all tablets have been 
File java/kudu-client/src/main/java/org/apache/kudu/client/

PS1, Line 1380:       if (oldRt != null) {
              :         // someone beat us to it
              :         continue;
> Done
I think you missed this.
File java/kudu-client/src/main/java/org/apache/kudu/client/

Line 349:   public Deferred alterTable(String name, AlterTableOptions ato) {
Why drop the generic type?

Line 356:       // Clear the caches so the new partition is immediately visible.
Nit: update this comment.

Line 1350:     // already be present
Nit: terminate with period.

Line 1361:     // Build of the list of discovered remote tablet instances. If 
we have
Nit: "Build the list..."

Line 1362:     // already discovered the tablet, it's locations are refreshed.
Nit: "its"

Line 1365:       // Early creating the tablet so that it parses out the pb
Nit: terminate with period.

Line 1370:       RemoteTablet currentTablet = tablet2client.get(tabletId);
As we discussed, it looks like there's an opportunity to get rid of 
tablet2client entirely, provided 1) we replace this lookup with a lookup into 
the main table locations cache, 2) adjust the locking so that there's no chance 
of a race between this lookup and the innards of cacheTabletLocations().

Line 170:       if (floorEntry != null &&
> Line 167 is limiting the removed set to the suffix after the lower bound, w
I see. I missed that this line acts on overlappingEntries, not entries.

Line 200:   public static class Entry {
> If you insist I can change it, but I think DeadlineTrackers are overkill fo
I don't feel strongly, no.
File java/kudu-client/src/test/java/org/apache/kudu/client/

Line 282:     String tableName = name.getMethodName() + 
> Done
Thanks. Could you fix it for the other tests in this file too?

To view, visit
To unsubscribe, visit

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

Reply via email to