[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
David Ribeiro Alves has posted comments on this change. Change subject: tablet: change default bloom filter FP rate to 0.01% .. Patch Set 2: Code-Review+2 thanks for adding the additional info -- 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-HasComments: No
[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
Jean-Daniel Cryans has posted comments on this change. Change subject: tablet: change default bloom filter FP rate to 0.01% .. Patch Set 2: Code-Review+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 end. -- 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-HasComments: Yes
[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
Kudu Jenkins has posted comments on this change. Change subject: tablet: change default bloom filter FP rate to 0.01% .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/2079/ -- 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-HasComments: No
[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
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] KUDU-1309: [java client] support tables with non-covering partition-key ranges
Adar Dembo has posted comments on this change. Change subject: KUDU-1309: [java client] support tables with non-covering partition-key ranges .. Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/3388/4/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java: Line 810: } else { This will need a rebase + conflict resolution. Line 1152: Map.Entry nonCoveredRange = getNonCoveredRange(table.getTableId(), key); Can use tableId here. http://gerrit.cloudera.org:8080/#/c/3388/4/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java: Line 327: operation.callback(response); Does this invoke the user callback eventually? If so, why do it before adding the error to the error collector? PS4, Line 784: private Object tablet = null; Ugh. Can you add more color on why this approach was necessary? Why can't we store the results in two separate fields, one LocatedTablet and one exception? http://gerrit.cloudera.org:8080/#/c/3388/2/java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeCache.java File java/kudu-client/src/main/java/org/kududb/client/NonCoveredRangeCache.java: Line 84: @Override > if concurrent updates happen to the entries the iteration is undefined, so Really? I thought ConcurrentSkipListMap defines an iteration order in the face of concurrent mutations. Javadoc says: "Insertion, removal, update, and access operations safely execute concurrently by multiple threads. Iterators are weakly consistent, returning elements reflecting the state of the map at some point at or since the creation of the iterator. They do not throw ConcurrentModificationException, and may proceed concurrently with other operations." Line 89: for (Map.Entry range : nonCoveredRanges.entrySet()) { > If you used a Guava Joiner to convert this list of Strings into a single co No love for Joiner? Bear in mind size() is more expensive on a ConcurrentSkipListMap. http://gerrit.cloudera.org:8080/#/c/3388/2/java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java File java/kudu-client/src/test/java/org/kududb/client/TestKuduSession.java: Line 221: public void testInsertManualFlushNonCoveredRange() throws Exception { > Replicas aren't important for these particular tests. Additionally, they c Right, but I don't think the other tests in this class particularly care about replication either (testUpsert() for example). Could you make a pass over all of the TestKuduSession tests and switch every test to single-replica, if it makes sense? It's more cognitive load as it is now; the reader's eyes are drawn to the inconsistency. -- To view, visit http://gerrit.cloudera.org:8080/3388 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id65a1f8a95bb16fc0360a17021a391afd3c9d03f Gerrit-PatchSet: 4 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-HasComments: Yes
[kudu-CR] locks: change kudu::shared lock constructor to pass by ref
Adar Dembo has posted comments on this change. Change subject: locks: change kudu::shared_lock constructor to pass by ref .. Patch Set 5: Verified+1 There was a build failure in the RELEASE build: 01:36:24 Linking CXX shared library ../../../lib/libkrpc.so 01:36:25 CMakeFiles/krpc.dir/constants.cc.o: file not recognized: File truncated 01:36:25 collect2: error: ld returned 1 exit status 01:36:25 make[2]: *** [lib/libkrpc.so] Error 1 01:36:25 make[1]: *** [src/kudu/rpc/CMakeFiles/krpc.dir/all] Error 2 01:36:25 make[1]: *** Waiting for unfinished jobs That file isn't touched by this patch, all of the other builds passed, and subsequent RELEASE builds (building off of this patch) have also passed, so I'm overriding Jenkins. -- 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: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] locks: switch from boost::shared mutex to new read-write mutex
Adar Dembo has posted comments on this change. Change subject: locks: switch from boost::shared_mutex to new read-write mutex .. Patch Set 5: Verified+1 Overriding Jenkins, flaky ksck timeout test. -- To view, visit http://gerrit.cloudera.org:8080/3499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b4b1df267468dc8c88c3de8e0345c61900576de Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [java-client] refactor AsyncKuduSession
Adar Dembo has posted comments on this change. Change subject: [java-client] refactor AsyncKuduSession .. Patch Set 3: (16 comments) Achievement unlocked: asynchronous programming wizard. http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java: Line 1410:Arrays.copyOf(partitionKey, partitionKey.length + 1), deadline); So this is how we obtain the very next value following partitionKey, right? Surprised it's so simple given how complicated EncodedKey::IncrementEncodedKey() is. http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java: Line 123:* application is {@link #apply}ing operations to one buffer (the {@code activeBuffer}), the Nit: {@link #apply} or {@code apply}? I see both here, not sure which is more correct. Line 131: private Buffer activeBuffer; Like the C++ client's batcher abstraction? Nice. Line 283: return flush(); Why flush() if the session is already closed? PS3, Line 350: buffer.callbackFlushNotification(); : inactiveBuffers.add(buffer); : flushNotification.getAndSet(new Deferred()).callback(null); And here there's the same sequence of events as in doFlush(), but in a different order. Why? PS3, Line 402: inactiveBuffers.add(buffer); : // Send buffer available notification. : flushNotification.getAndSet(new Deferred()).callback(null); : buffer.callbackFlushNotification(); AFAICT, there's no lock held for this work. The order strikes me as strange: we're adding buffer to inactiveBuffers (thus making it globally visible) before invoking callbacks, especially the buffer's own callback. Is this safe? Line 409: Deferred> batchResponses = new Deferred<>(); Nit: not really a fan of sometimes using Deferred.fromResult() and other times invoking the Deferred constructor directly. Can we be more consistent? PS3, Line 425: private static final ConvertBatchToListOfResponsesCB INSTANCE = : new ConvertBatchToListOfResponsesCB(); Is this for correctness or performance? Seems like the implementation would be simpler if we didn't maintain a singleton. Line 563: doFlush(fullBuffer); What happens if doFlush() throws but we're executing it because we throw a PleaseThrottle or NonRecoverable exception in flush()? Line 704: private final class Buffer { Shouldn't Buffer be declared static? Or did you intend for it to be an inner class? I don't see any accesses to the outer class; maybe I missed them though. Nit: not really clear on why you'd use final here. I don't think there's any real danger of someone extending Buffer, so final just adds noise, no? Line 748: flushNotification = new Deferred<>(); Nit: use fromResult(null) here to be consistent with the inline field constructor. Better yet, perhaps add a no-arg constructor that initializes via reset()? Then it's even more consistent. Line 756: .add("flusherTask", flusherTask) This will just be an object address, right? Useful? http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/Batch.java File java/kudu-client/src/main/java/org/kududb/client/Batch.java: Line 34: import org.kududb.tserver.Tserver; Odd that this comes after Tserver.TabletServerErrorPB. Line 46: final List operations = new ArrayList<>(1250); What's magical about 1250? http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/client/Operation.java File java/kudu-client/src/main/java/org/kududb/client/Operation.java: Line 203: static Tserver.WriteRequestPB.Builder createAndFillWriteRequestPB(List operations) { What motivated the changes here from variadic args to lists? Isn't the former (marginally) more performant? http://gerrit.cloudera.org:8080/#/c/3477/3/java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java File java/kudu-client/src/main/java/org/kududb/util/AsyncUtil.java: Line 22: import java.util.List; Unused? -- 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: 3 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-HasComments: Yes
[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
David Ribeiro Alves has posted comments on this change. Change subject: tablet: change default bloom filter FP rate to 0.01% .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3517/1/docs/release_notes.adoc File docs/release_notes.adoc: PS1, Line 72: substantially how substantantially? PS1, Line 74: incremental which increase? Can you provide rough numbers for both? -- 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: 1 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-HasComments: Yes
[kudu-CR] Persistent cache support for NVM
Sarah Jelinek has posted comments on this change. Change subject: Persistent cache support for NVM .. Patch Set 16: (1 comment) Pushed all code review changes. http://gerrit.cloudera.org:8080/#/c/2593/15/src/kudu/util/nvm_cache.cc File src/kudu/util/nvm_cache.cc: Line 54: TAG_FLAG(nvm_cache_path, experimental); > The reason for this is that when operating in persistent mode the user has I died modify this to be nvm_cache_pool to indicate that when persistent memory is in use a pool with a specified name is created. Otherwise it's just a directory where a pool is created. -- To view, visit http://gerrit.cloudera.org:8080/2593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id72570ead662670bf42175756a18ae08d7cd0a07 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sarah Jelinek Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sarah Jelinek Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Persistent cache support for NVM
Kudu Jenkins has posted comments on this change. Change subject: Persistent cache support for NVM .. Patch Set 16: Build Started http://104.196.14.100/job/kudu-gerrit/2078/ -- To view, visit http://gerrit.cloudera.org:8080/2593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id72570ead662670bf42175756a18ae08d7cd0a07 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sarah Jelinek Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sarah Jelinek Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Persistent cache support for NVM
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/2593 to look at the new patch set (#16). Change subject: Persistent cache support for NVM .. Persistent cache support for NVM Change-Id: Id72570ead662670bf42175756a18ae08d7cd0a07 --- M build-support/run-test.sh M src/kudu/cfile/block_cache.h M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/cfile_reader.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/cache-test.cc M src/kudu/util/cache.cc M src/kudu/util/cache.h A src/kudu/util/nvm_cache-test.cc M src/kudu/util/nvm_cache.cc 10 files changed, 713 insertions(+), 201 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/2593/16 -- To view, visit http://gerrit.cloudera.org:8080/2593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id72570ead662670bf42175756a18ae08d7cd0a07 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sarah Jelinek Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sarah Jelinek Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
Jean-Daniel Cryans has posted comments on this change. Change subject: tablet: change default bloom filter FP rate to 0.01% .. Patch Set 1: Code-Review+2 -- 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: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
Kudu Jenkins has posted comments on this change. Change subject: tablet: change default bloom filter FP rate to 0.01% .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2077/ -- 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: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] tablet: change default bloom filter FP rate to 0.01%
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] locks: switch from boost::shared mutex to new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_mutex to new read-write mutex .. Patch Set 5: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2074/ -- To view, visit http://gerrit.cloudera.org:8080/3499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b4b1df267468dc8c88c3de8e0345c61900576de Gerrit-PatchSet: 5 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] locks: add new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 5: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2075/ -- 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] locks: switch from boost::shared lock to kudu::shared lock
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_lock to kudu::shared_lock .. Patch Set 5: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2073/ -- To view, visit http://gerrit.cloudera.org:8080/3498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I523cc09345b50f7ad9e6af0180493774246a9bf8 Gerrit-PatchSet: 5 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] locks: change kudu::shared lock constructor to pass by ref
Kudu Jenkins has posted comments on this change. Change subject: locks: change kudu::shared_lock constructor to pass by ref .. Patch Set 5: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2076/ -- 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: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: add boost and switch to header-only build
Kudu Jenkins has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 4: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2072/ -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 4 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] locks: add new read-write mutex
Adar Dembo has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 4: (1 comment) > (1 comment) > > LGTM with some concern regarding error detection if something goes > wrong. > > So, we are about to catch some those non-run-time issues like > trying to acquire the lock held by the same thread, etc. in debug > mode, which is OK. > > Do we expect to catch some run-time issues like lack of shared > memory to accommodate a new lock, etc.? I.e. we are not > propagating errors from pthread_rwlock_xxx() functions in release > mode at all (if I'm not missing something). Does it make sense to > throw some sort of exception for run-time errors (like > std::runtime_error)? It probably does (I'm not 100% sure, would need to give it more thought), but I'd rather keep that out of scope of this patch. The behavior of this new lock is consistent with that of our other pthread-based locks. If we're going to address the issue of runtime checking of pthread return values, I think it makes sense to do it across the board in a separate patch. http://gerrit.cloudera.org:8080/#/c/3496/4/src/kudu/util/mutex.cc File src/kudu/util/mutex.cc: Line 92: ; // NOLINT(whitespace/semicolon) > As Dan already mentioned: is it possible to keep the semicolon under the if Yes, but I actually messed up when I published this version of the diff. I meant to pull the DCHECK_EQ() out of the NDEBUG block. See the next revision. If it's pulled out, it's no longer possible to put the semicolon on the end of the line, at least not without duplicating the DCHECK_EQ(). -- 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: 4 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: Yes
[kudu-CR] locks: add new read-write mutex
Alexey Serbin has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 2: (1 comment) LGTM with some concern regarding error detection if something goes wrong. So, we are about to catch some those non-run-time issues like trying to acquire the lock held by the same thread, etc. in debug mode, which is OK. Do we expect to catch some run-time issues like lack of shared memory to accommodate a new lock, etc.? I.e. we are not propagating errors from pthread_rwlock_xxx() functions in release mode at all (if I'm not missing something). Does it make sense to throw some sort of exception for run-time errors (like std::runtime_error)? http://gerrit.cloudera.org:8080/#/c/3496/4/src/kudu/util/mutex.cc File src/kudu/util/mutex.cc: Line 92: } As Dan already mentioned: is it possible to keep the semicolon under the ifndef and in the same line as last statement? -- 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: 2 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: Yes
[kudu-CR] locks: switch from boost::shared lock to kudu::shared lock
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_lock to kudu::shared_lock .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2070/ -- To view, visit http://gerrit.cloudera.org:8080/3498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I523cc09345b50f7ad9e6af0180493774246a9bf8 Gerrit-PatchSet: 5 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] locks: add new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2068/ -- 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] locks: change kudu::shared lock constructor to pass by ref
Kudu Jenkins has posted comments on this change. Change subject: locks: change kudu::shared_lock constructor to pass by ref .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2067/ -- 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: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] thirdparty: add boost and switch to header-only build
Kudu Jenkins has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2071/ -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 4 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] locks: switch from boost::shared mutex to new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_mutex to new read-write mutex .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2069/ -- To view, visit http://gerrit.cloudera.org:8080/3499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b4b1df267468dc8c88c3de8e0345c61900576de Gerrit-PatchSet: 5 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] locks: add new read-write mutex
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3496 to look at the new patch set (#5). Change subject: locks: add new read-write mutex .. locks: add new read-write mutex The new mutex is a thin wrapper around pthread read/write locks. It has no features of which to speak: no debugging hooks, no optimizations, nothing. Take these rwlock-perf results with a grain of salt; they only test read/read contention, not read/write or write/write contention. The values are millions of cycles. num_threads laptop_old laptop_new ve0518_old ve0518_new --- 1 26183 18982 2 1604 644914730 3 1504 9172204 1567 4 2398 1792 3185 2162 5 3210 2179 3943 2308 6 4070 2696 4135 2515 7 4741 3253 4557 2732 8 5457 3853 5114 3145 Change-Id: I5462e69291fb9498ebd4aaa1728c64658667aa4b --- M src/kudu/experiments/rwlock-perf.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/mutex.cc A src/kudu/util/rw_mutex.cc A src/kudu/util/rw_mutex.h 5 files changed, 122 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/3496/5 -- To view, visit http://gerrit.cloudera.org:8080/3496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset 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
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Jean-Daniel Cryans has submitted this change and it was merged. Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. KUDU-1496. NoLeaderMasterFoundException are mishandled This is a regression from 0c8f72b4c5d637dba4c0c0da4f55ea918c287d88. Once we have multi-master unit tests re-enabled, we'll have this covered. Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Reviewed-on: http://gerrit.cloudera.org:8080/3516 Reviewed-by: Adar Dembo Reviewed-by: Dan Burkert Tested-by: Jean-Daniel Cryans --- M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java 1 file changed, 6 insertions(+), 5 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Jean-Daniel Cryans: Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. Patch Set 1: Verified+1 Forcing +1 due to current messed up workspace. -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: No
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Dan Burkert has posted comments on this change. Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] thirdparty: add boost and switch to header-only build
Kudu Jenkins has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2066/ -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 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: No
[kudu-CR] locks: switch from boost::shared mutex to new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_mutex to new read-write mutex .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2064/ -- To view, visit http://gerrit.cloudera.org:8080/3499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b4b1df267468dc8c88c3de8e0345c61900576de Gerrit-PatchSet: 4 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] locks: add new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2063/ -- 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: 4 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] locks: switch from boost::shared lock to kudu::shared lock
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_lock to kudu::shared_lock .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2065/ -- To view, visit http://gerrit.cloudera.org:8080/3498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I523cc09345b50f7ad9e6af0180493774246a9bf8 Gerrit-PatchSet: 4 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] locks: change kudu::shared lock constructor to pass by ref
Kudu Jenkins has posted comments on this change. Change subject: locks: change kudu::shared_lock constructor to pass by ref .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2062/ -- 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: 4 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] locks: add new read-write mutex
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3496 to look at the new patch set (#4). Change subject: locks: add new read-write mutex .. locks: add new read-write mutex The new mutex is a thin wrapper around pthread read/write locks. It has no features of which to speak: no debugging hooks, no optimizations, nothing. Take these rwlock-perf results with a grain of salt; they only test read/read contention, not read/write or write/write contention. The values are millions of cycles. num_threads laptop_old laptop_new ve0518_old ve0518_new --- 1 26183 18982 2 1604 644914730 3 1504 9172204 1567 4 2398 1792 3185 2162 5 3210 2179 3943 2308 6 4070 2696 4135 2515 7 4741 3253 4557 2732 8 5457 3853 5114 3145 Change-Id: I5462e69291fb9498ebd4aaa1728c64658667aa4b --- M src/kudu/experiments/rwlock-perf.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/mutex.cc A src/kudu/util/rw_mutex.cc A src/kudu/util/rw_mutex.h 5 files changed, 122 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/3496/4 -- To view, visit http://gerrit.cloudera.org:8080/3496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5462e69291fb9498ebd4aaa1728c64658667aa4b Gerrit-PatchSet: 4 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
[kudu-CR] thirdparty: add boost and switch to header-only build
Dan Burkert has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Jean-Daniel Cryans has posted comments on this change. Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. Patch Set 1: (1 comment) > Oops. > > Outside of multi-master, this only manifests when the sole master > is booting up, right? That would explain why it happens often in > our unit tests. Yup, I just didn't want to write tests that we already have. http://gerrit.cloudera.org:8080/#/c/3516/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java: Line 814: return d; > is it important that the getDeferred call happen before delayedSendRpcToTab If by some chance the RPC completed before the return is done, we'd be sending back a new Deferred. Almost impossible to run into, but hey... -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Dan Burkert has posted comments on this change. Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3516/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java: Line 814: return d; is it important that the getDeferred call happen before delayedSendRpcToTablet? If not I would suggest directly returning request.getDeferred(). -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Adar Dembo has posted comments on this change. Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. Patch Set 1: Code-Review+2 Oops. Outside of multi-master, this only manifests when the sole master is booting up, right? That would explain why it happens often in our unit tests. -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Binglin Chang Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Jean-Daniel Cryans has uploaded a new change for review. http://gerrit.cloudera.org:8080/3516 Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. KUDU-1496. NoLeaderMasterFoundException are mishandled This is a regression from 0c8f72b4c5d637dba4c0c0da4f55ea918c287d88. Once we have multi-master unit tests re-enabled, we'll have this covered. Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 --- M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java 1 file changed, 6 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/3516/1 -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans
[kudu-CR] KUDU-1496. NoLeaderMasterFoundException are mishandled
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1496. NoLeaderMasterFoundException are mishandled .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2061/ -- To view, visit http://gerrit.cloudera.org:8080/3516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie33410bdcfcc1bf9d3afb559a01f50538d7e10b5 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Add a ToString() method to Proxy
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 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 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
Jean-Daniel Cryans has posted comments on this change. Change subject: Add 6/27 weekly update .. Patch Set 2: Code-Review+2 -- 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-HasComments: No
[kudu-CR](gh-pages) Add 6/27 weekly update
Todd Lipcon has uploaded a new patch set (#2). 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/2 -- To view, visit http://gerrit.cloudera.org:8080/3515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6ce4bf6e02a5481bf3e032d3345d7afc93c244a2 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR](gh-pages) Add 6/27 weekly update
Jean-Daniel Cryans has posted comments on this change. Change subject: Add 6/27 weekly update .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3515/1/_posts/2016-06-27-weekly-update.md File _posts/2016-06-27-weekly-update.md: PS1, Line 24: replication replicating? -- 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: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-HasComments: Yes
[kudu-CR] thirdparty: add boost and switch to header-only build
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3500 to look at the new patch set (#2). Change subject: thirdparty: add boost and switch to header-only build .. thirdparty: add boost and switch to header-only build The addition of boost to thirdparty is bound to make some squeamish, so here's my rationale: 1. My original goal was to enable the TSAN build on Ubuntu 16.04, a distro that has switched to the new gcc5 ABI. This renders the system boost installation unusable as it can't link against our TSAN-instrumented libstdc++. To be fair, switching to a header-only build has eliminated this problem. 2. Boost is effectively the last remaining dependency that isn't explicitly managed as a thirdparty dependency. The only exceptions are glibc, libstdc++, and libsasl, all of which should not be managed due to their interactions with other ABIs. In my opinion, Kudu should manage as many dependencies as possible in this way. 3. Doing this means Kudu developers will no longer need to install boost system packages to build Kudu. 4. Historically, Kudu has permitted the use of any boost version even though some versions needed patches to build with a new compiler. IIRC, https://svn.boost.org/trac/boost/ticket/6165 was one such instance. 5. For the boost logic that Kudu does use, we'll know exactly which features exist and which do not. There'll be no more version-based ambiguity. 6. Doing this means we can drop the odd boost_uuid appendage. 7. This patch by no means implies that boost is to stay in Kudu forever. We're already at the point where we can get by with boost headers alone. Once we replace the remaining bits (i.e. optional and intrusive lists come to mind), we can jettison it altogether. With that out of the way, here are the details: - Inclusion of boost as a thirdparty dependency. I spent some time fighting with the boost build to get it to copy the headers without building anything; eventually I gave up and used rsync instead. - Removal of boost components from Kudu's cmake logic. This netted a nice cleanup of cmake code. - Removal of boost handling from documentation and other random places. - Removal of boost_uuid. Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 --- M CMakeLists.txt M LICENSE.txt M build-support/dist_test.py M build-support/release/rat_exclude_files.txt M docs/contributing.adoc M docs/installation.adoc M src/kudu/util/CMakeLists.txt M thirdparty/.gitignore M thirdparty/LICENSE.txt D thirdparty/boost_uuid/LICENSE.txt D thirdparty/boost_uuid/boost/uuid/name_generator.hpp D thirdparty/boost_uuid/boost/uuid/nil_generator.hpp D thirdparty/boost_uuid/boost/uuid/random_generator.hpp D thirdparty/boost_uuid/boost/uuid/seed_rng.hpp D thirdparty/boost_uuid/boost/uuid/sha1.hpp D thirdparty/boost_uuid/boost/uuid/string_generator.hpp D thirdparty/boost_uuid/boost/uuid/uuid.hpp D thirdparty/boost_uuid/boost/uuid/uuid_generators.hpp D thirdparty/boost_uuid/boost/uuid/uuid_io.hpp D thirdparty/boost_uuid/boost/uuid/uuid_serialize.hpp M thirdparty/build-definitions.sh M thirdparty/build-thirdparty.sh M thirdparty/download-thirdparty.sh A thirdparty/patches/boost-issue-12179-fix-compilation-errors.patch M thirdparty/preflight.py M thirdparty/vars.sh 26 files changed, 98 insertions(+), 1,558 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/3500/2 -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] locks: switch from boost::shared mutex to new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_mutex to new read-write mutex .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2058/ -- To view, visit http://gerrit.cloudera.org:8080/3499 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1b4b1df267468dc8c88c3de8e0345c61900576de Gerrit-PatchSet: 3 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] locks: change kudu::shared lock constructor to pass by ref
Hello Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3497 to look at the new patch set (#3). Change subject: locks: change kudu::shared_lock constructor to pass by ref .. locks: change kudu::shared_lock constructor to pass by ref This is less Kudu-style friendly, but it'll make the (eventual) transition to the C++14 std::shared_lock smoother, and it's also more consistent with std::lock_guard. Change-Id: I1e25a4f519f5cc09f08f8aeda5284eabdadd0c46 --- M src/kudu/client/meta_cache.cc M src/kudu/fs/block_manager-stress-test.cc M src/kudu/fs/log_block_manager.cc M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/rpcz_store.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/transactions/write_transaction.cc M src/kudu/util/locks.h 9 files changed, 20 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/3497/3 -- To view, visit http://gerrit.cloudera.org:8080/3497 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1e25a4f519f5cc09f08f8aeda5284eabdadd0c46 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
[kudu-CR] locks: switch from boost::shared lock to kudu::shared lock
Kudu Jenkins has posted comments on this change. Change subject: locks: switch from boost::shared_lock to kudu::shared_lock .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2059/ -- To view, visit http://gerrit.cloudera.org:8080/3498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I523cc09345b50f7ad9e6af0180493774246a9bf8 Gerrit-PatchSet: 3 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] locks: change kudu::shared lock constructor to pass by ref
Kudu Jenkins has posted comments on this change. Change subject: locks: change kudu::shared_lock constructor to pass by ref .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2056/ -- 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: 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: No
[kudu-CR] locks: add new read-write mutex
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3496 to look at the new patch set (#3). Change subject: locks: add new read-write mutex .. locks: add new read-write mutex The new mutex is a thin wrapper around pthread read/write locks. It has no features of which to speak: no debugging hooks, no optimizations, nothing. Take these rwlock-perf results with a grain of salt; they only test read/read contention, not read/write or write/write contention. The values are millions of cycles. num_threads laptop_old laptop_new ve0518_old ve0518_new --- 1 26183 18982 2 1604 644914730 3 1504 9172204 1567 4 2398 1792 3185 2162 5 3210 2179 3943 2308 6 4070 2696 4135 2515 7 4741 3253 4557 2732 8 5457 3853 5114 3145 Change-Id: I5462e69291fb9498ebd4aaa1728c64658667aa4b --- M src/kudu/experiments/rwlock-perf.cc M src/kudu/util/CMakeLists.txt M src/kudu/util/mutex.cc A src/kudu/util/rw_mutex.cc A src/kudu/util/rw_mutex.h 5 files changed, 122 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/3496/3 -- To view, visit http://gerrit.cloudera.org:8080/3496 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5462e69291fb9498ebd4aaa1728c64658667aa4b Gerrit-PatchSet: 3 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
[kudu-CR] locks: add new read-write mutex
Kudu Jenkins has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2057/ -- 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: 3 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] thirdparty: add boost and switch to header-only build
Kudu Jenkins has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/2060/ -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 2 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](gh-pages) Add 6/27 weekly update
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 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
[kudu-CR] Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange
Todd Lipcon has posted comments on this change. Change subject: Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibb5875e8d4200b0058cf7e7c4ee04771b91d2a24 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange
Todd Lipcon has posted comments on this change. Change subject: Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange .. Patch Set 1: A java test failed due to https://issues.apache.org/jira/browse/KUDU-1496 (unrelated) -- To view, visit http://gerrit.cloudera.org:8080/3511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibb5875e8d4200b0058cf7e7c4ee04771b91d2a24 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Implement kudu::optional replacement for boost::optional
Mike Percy has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: > Also not sure of the purpose of the extra red zones (ASAN > would already insert those around the object, right?) I didn't think ASAN did this for stack-allocated objects. Does it? If so, the red zones should be removed from this impl. -- 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
[kudu-CR] Implement kudu::optional replacement for boost::optional
Todd Lipcon has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: The boost implementation already has ASSERTs for accessing it while not initialized, so the additional value of the asan stuff is not huge IMO. Also not sure of the purpose of the extra red zones (ASAN would already insert those around the object, right?) Anyway I'm not going to block this if everyone else thinks this is a good idea, it just seems like complicated stuff to get right, and I'd rather trust boost:: or std:: (in some future c++17 world) to do it for us. -- 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
[kudu-CR] Implement kudu::optional replacement for boost::optional
Mike Percy has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: > I would like some careful eyes on this before we merge it, as the > move semantics related stuff that is being done is at the edge of > my understanding of C++11. Not presupposing that we will merge it, but if we do. -- 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
[kudu-CR] Implement kudu::optional replacement for boost::optional
Mike Percy has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: > Well, the call sites are going to stay the same, but we're taking > on a few hundred lines of this complicated C++ stuff that we had to > write ourselves (and thus has a higher chance of bugs vs code taken > from a mature project like Chromium). Yeah, the implementation turned out to require more complicated C++ features than I had originally envisioned, because of the move semantics and the need to support nearly any type. It still doesn't support holding references (although the need for that is arguable). I would like some careful eyes on this before we merge it, as the move semantics related stuff that is being done is at the edge of my understanding of C++11. > The other cases where we have re-implemented something like this > are primarily in synchronization where the benefit has been > improved instrumentation/debuggability (like tracing integration > for mutex acquisition time). I don't see any such possibilities > with something like optional. Actually I was able to incorporate some ASAN features into this implementation, including poisoning and a red zone. It could potentially help to catch memory-related bugs, even when it's stack-allocated. -- 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
[kudu-CR] Allow to force-override color diagnostics
Adar Dembo has posted comments on this change. Change subject: Allow to force-override color diagnostics .. Patch Set 1: Code-Review+2 Sounds like there's a reasonable use case for it. -- To view, visit http://gerrit.cloudera.org:8080/3509 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c19651df6d74753b0d4e8ac85ff98326dee7b10 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: No
[kudu-CR] Implement kudu::optional replacement for boost::optional
Todd Lipcon has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: > This is meant to completely replace our usage of boost::optional so it should > be net zero in terms of new code used vs. old code dropped, or even net > negative given that I don't think Mike reimplemented all of boost::optional. > Besides, you've been open to incorporating Chromium code in the past to > replace boost usage (e.g. Chromium callback/bind). Is this terribly different? Well, the call sites are going to stay the same, but we're taking on a few hundred lines of this complicated C++ stuff that we had to write ourselves (and thus has a higher chance of bugs vs code taken from a mature project like Chromium). Callback/Bind specifically also supported the chromium refptrs which don't work so well with boost::bind (though if we decided to move towards shared_ptr and std::bind in the future I think that could be good). The other cases where we have re-implemented something like this are primarily in synchronization where the benefit has been improved instrumentation/debuggability (like tracing integration for mutex acquisition time). I don't see any such possibilities with something like optional. -- 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
[kudu-CR] thirdparty: add boost and switch to header-only build
Adar Dembo has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3500/1/docs/contributing.adoc File docs/contributing.adoc: Line 151: We are in the process of removing all remaining `boost` dependencies from the > Yes, but additionally I think boost::optional is a powerful tool for writin Alright, then I'll update this to say something to the effect of: 1. We're actively trying to remove boost. 2. We're using boost in a header-only capacity right now; don't introduce any library dependencies. 3. Please don't introduce dependencies on new boost header-only features. 4. You can use existing boost header-only features on which we already depend (e.g. boost::optional, boost::bind). These features may be removed in the future; when using boost, please be mindful of the "current state" of the world (e.g. boost::bind might be gone soon). -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 1 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] ts recovery-itest: fix flakiness in TestRestartWithPendingCommitFromFailedOp
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3514 to review the following change. Change subject: ts_recovery-itest: fix flakiness in TestRestartWithPendingCommitFromFailedOp .. ts_recovery-itest: fix flakiness in TestRestartWithPendingCommitFromFailedOp This test seems to be flaky due to not waiting long enough for the TS to crash. I tried to repro on dist-test, but the GCE test slaves are fast enough that it passed 1000/1000. On EC2 apparently things are a little slower so 10 seconds isn't enough time to guarantee that the injected fault fires. Change-Id: I60db2acf7642aa6eb2c850032a7dc9639b6ae40e --- M src/kudu/integration-tests/ts_recovery-itest.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/3514/1 -- To view, visit http://gerrit.cloudera.org:8080/3514 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I60db2acf7642aa6eb2c850032a7dc9639b6ae40e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo
[kudu-CR] Implement kudu::optional replacement for boost::optional
Mike Percy has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3512/1/src/kudu/util/optional.h File src/kudu/util/optional.h: Line 237: const T& GetValueAsConstRef() const& { return *GetValueAsConstPtr(); } > So there's a difference between a "const" method and a "const&" method? Did The & allows it to bind to an rvalue object as well as an lvalue object. -- 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: Yes
[kudu-CR] ts recovery-itest: fix flakiness in TestRestartWithPendingCommitFromFailedOp
Kudu Jenkins has posted comments on this change. Change subject: ts_recovery-itest: fix flakiness in TestRestartWithPendingCommitFromFailedOp .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2055/ -- To view, visit http://gerrit.cloudera.org:8080/3514 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I60db2acf7642aa6eb2c850032a7dc9639b6ae40e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Implement kudu::optional replacement for boost::optional
Adar Dembo has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: > I don't see the purpose of taking on this extra code. Code's a > liability, not an asset, and all that. This is meant to completely replace our usage of boost::optional so it should be net zero in terms of new code used vs. old code dropped, or even net negative given that I don't think Mike reimplemented all of boost::optional. Besides, you've been open to incorporating Chromium code in the past to replace boost usage (e.g. Chromium callback/bind). Is this terribly different? I think there are reasonable counter-arguments to be made (e.g. boost::optional was probably written/tested by people with deeper C++ knowledge than ourselves), but I don't think this one is that convincing. -- 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: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Implement kudu::optional replacement for boost::optional
Adar Dembo has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: (3 comments) I only skimmed this; I think a real review requires a level of C++ knowledge that I don't think I have. http://gerrit.cloudera.org:8080/#/c/3512/1/src/kudu/gutil/dynamic_annotations.h File src/kudu/gutil/dynamic_annotations.h: Line 64: #include // For size_t. Technically size_t is defined in stddef.h (or cstddef, if you prefer a C++ header). http://gerrit.cloudera.org:8080/#/c/3512/1/src/kudu/util/CMakeLists.txt File src/kudu/util/CMakeLists.txt: Line 297: ADD_KUDU_TEST(optional-test) Nit: precedes os-util-test alphabetically. http://gerrit.cloudera.org:8080/#/c/3512/1/src/kudu/util/optional.h File src/kudu/util/optional.h: Line 237: const T& GetValueAsConstRef() const& { return *GetValueAsConstPtr(); } So there's a difference between a "const" method and a "const&" method? Didn't know that. What is the difference? -- 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: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Migrate from boost::optional to kudu::optional
Mike Percy has posted comments on this change. Change subject: Migrate from boost::optional to kudu::optional .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/3513/1/src/kudu/tablet/memrowset.cc File src/kudu/tablet/memrowset.cc: Line 411: exclusive_upper_bound_ = spec->exclusive_upper_bound_key()->encoded_key(); Need to check w/ Todd on this but I think it should be fine to do it. http://gerrit.cloudera.org:8080/#/c/3513/1/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: Line 670: return; } Oops, inadvertent change, need to restore the newline before this brace. http://gerrit.cloudera.org:8080/#/c/3513/1/src/kudu/tserver/ts_tablet_manager.cc File src/kudu/tserver/ts_tablet_manager.cc: Line 993: Status delete_status = DeleteTabletData(meta, TABLET_DATA_TOMBSTONED, optional()); This should probably be replaced with nullopt -- To view, visit http://gerrit.cloudera.org:8080/3513 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia46ba817d507af8b26add764dc3859364d15e0da 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-HasComments: Yes
[kudu-CR] Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange
Kudu Jenkins has posted comments on this change. Change subject: Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange .. Patch Set 1: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2054/ -- To view, visit http://gerrit.cloudera.org:8080/3511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibb5875e8d4200b0058cf7e7c4ee04771b91d2a24 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Allow to force-override color diagnostics
Todd Lipcon has posted comments on this change. Change subject: Allow to force-override color diagnostics .. Patch Set 1: I use something like this locally -- when I run 'ninja', it sometimes re-runs cmake for me, but the output is a pipe into ninja, rather than a TTY, which means that cmake will decide not to use color. My workaround is to put it in my ccache-clang wrapper though: #!/bin/bash -e if [ -n "$CLANG_ALWAYS_COLOR" ] || test -t 2 ; then color_flags="-fcolor-diagnostics" fi CCACHE_CPP2=yes exec ccache clang++ -Qunused-arguments $color_flags "$@" -- To view, visit http://gerrit.cloudera.org:8080/3509 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c19651df6d74753b0d4e8ac85ff98326dee7b10 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: No
[kudu-CR] Implement kudu::optional replacement for boost::optional
Todd Lipcon has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: Code-Review-1 I don't see the purpose of taking on this extra code. Code's a liability, not an asset, and all that. -- 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: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Allow to force-override color diagnostics
Mike Percy has posted comments on this change. Change subject: Allow to force-override color diagnostics .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3509/1/CMakeLists.txt File CMakeLists.txt: Line 230: # We also provide a manual override as -DKUDU_FORCE_COLOR_DIAGNOSTICS=1. > Curious: what's the motivation for the override? How is the existing heuris I looked into it a little bit. I'm not exactly sure why it's failing for me. I wonder if it has to do with bash aliases and functions. But AFAICT I am not redirecting stderr. -- To view, visit http://gerrit.cloudera.org:8080/3509 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c19651df6d74753b0d4e8ac85ff98326dee7b10 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-HasComments: Yes
[kudu-CR] Update docs on how to run gcovr
Mike Percy has posted comments on this change. Change subject: Update docs on how to run gcovr .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/3508/1/README.adoc File README.adoc: Line 241: $ mkdir cov_html > Hmm, probably don't need this anymore. ah, right Line 244: --gcov-executable=$(pwd)/../../build-support/llvm-gcov-wrapper \ > Why do we need $(pwd)? We're in build/coverage; given the "blessed" build l I know it's lame but it doesn't work without an absolute path. I'm sure it's a bug in gcovr. This is the easiest way. Line 245: --html --html-details -o coverage.html > What happens if you run without -o? I'm not sure. I'll try it out. -- 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-HasComments: Yes
[kudu-CR] Allow to force-override color diagnostics
Adar Dembo has posted comments on this change. Change subject: Allow to force-override color diagnostics .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3509/1/CMakeLists.txt File CMakeLists.txt: Line 230: # We also provide a manual override as -DKUDU_FORCE_COLOR_DIAGNOSTICS=1. Curious: what's the motivation for the override? How is the existing heuristic failing for you? -- To view, visit http://gerrit.cloudera.org:8080/3509 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c19651df6d74753b0d4e8ac85ff98326dee7b10 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Migrate from boost::optional to kudu::optional
Kudu Jenkins has posted comments on this change. Change subject: Migrate from boost::optional to kudu::optional .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2053/ -- To view, visit http://gerrit.cloudera.org:8080/3513 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia46ba817d507af8b26add764dc3859364d15e0da Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] thirdparty: add boost and switch to header-only build
Dan Burkert has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3500/1/docs/contributing.adoc File docs/contributing.adoc: Line 151: We are in the process of removing all remaining `boost` dependencies from the > I really was hoping to avoid introducing new Boost usage, so that we stand Yes, but additionally I think boost::optional is a powerful tool for writing higher quality code, and its use should be encouraged wherever it makes sense from a code simplicity/maintainability perspective. Optional is especially important when working with value types, as is often possible with the new move semantics, and unlocks simpler and more performant solutions than the existing tools (e.g. nullable unique_ptr) in many cases. Basically, I think Optional is a great abstraction and to discourage its use because of a minor distaste of boost is going to do more harm than good. I don't care as much about intrusive, and would agree that we should not encourage it, only that it should be available in the very few cases where it is justified. I doubt it will get used again any time soon, so it's a bit of a non-issue. -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 1 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] Implement kudu::optional replacement for boost::optional
Kudu Jenkins has posted comments on this change. Change subject: Implement kudu::optional replacement for boost::optional .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2052/ -- 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: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Implement kudu::optional replacement for boost::optional
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3512 to review the following change. Change subject: Implement kudu::optional replacement for boost::optional .. Implement kudu::optional replacement for boost::optional This class attempts to implement as much of the C++17 specification of std::optional as is reasonably simple to provide in C++11. This class does not throw exceptions and value() is not implemented. Change-Id: Ib80c35cc9a4712572f85eeb7717e17869cd5e081 --- M src/kudu/gutil/dynamic_annotations.h M src/kudu/util/CMakeLists.txt A src/kudu/util/optional-test.cc A src/kudu/util/optional.h A src/kudu/util/optional_fwd.h 5 files changed, 518 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/12/3512/1 -- To view, visit http://gerrit.cloudera.org:8080/3512 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib80c35cc9a4712572f85eeb7717e17869cd5e081 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo
[kudu-CR] Migrate from boost::optional to kudu::optional
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3513 to review the following change. Change subject: Migrate from boost::optional to kudu::optional .. Migrate from boost::optional to kudu::optional Change-Id: Ia46ba817d507af8b26add764dc3859364d15e0da --- M src/kudu/client/client.cc M src/kudu/client/scan_predicate.cc M src/kudu/client/scan_token-internal.cc M src/kudu/client/table_alterer-internal.cc M src/kudu/client/table_alterer-internal.h M src/kudu/common/column_predicate.cc M src/kudu/common/column_predicate.h M src/kudu/common/partition-test.cc M src/kudu/common/partition_pruner-test.cc M src/kudu/common/partition_pruner.cc M src/kudu/common/wire_protocol.cc M src/kudu/common/wire_protocol.h M src/kudu/consensus/consensus.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h M src/kudu/integration-tests/client_failover-itest.cc M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/delete_table-test.cc M src/kudu/integration-tests/raft_consensus-itest.cc M src/kudu/integration-tests/remote_bootstrap-itest.cc M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/rpc/service_pool.cc M src/kudu/rpc/service_queue-test.cc M src/kudu/rpc/service_queue.cc M src/kudu/rpc/service_queue.h M src/kudu/tablet/composite-pushdown-test.cc M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tools/kudu-admin.cc M src/kudu/tserver/tablet_peer_lookup.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 38 files changed, 168 insertions(+), 173 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/3513/1 -- To view, visit http://gerrit.cloudera.org:8080/3513 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia46ba817d507af8b26add764dc3859364d15e0da Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo
[kudu-CR] Update docs on how to run gcovr
Adar Dembo has posted comments on this change. Change subject: Update docs on how to run gcovr .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/3508/1/README.adoc File README.adoc: Line 241: $ mkdir cov_html Hmm, probably don't need this anymore. Line 244: --gcov-executable=$(pwd)/../../build-support/llvm-gcov-wrapper \ Why do we need $(pwd)? We're in build/coverage; given the "blessed" build layout, isn't that guaranteed to be two subdirs deeper than build-support? Line 245: --html --html-details -o coverage.html What happens if you run without -o? -- 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-HasComments: Yes
[kudu-CR] thirdparty: add boost and switch to header-only build
Adar Dembo has posted comments on this change. Change subject: thirdparty: add boost and switch to header-only build .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/3500/1//COMMIT_MSG Commit Message: Line 18:libstdc++, and libsasl, all of which should not be managed due to their > and openssl, although the point still stands. Right, though I explicitly omitted openssl because, at least for now, you really can't configure Kudu to use it. PS1, Line 22: Some distros (e.g. SLES12) don't offer :system packages > The specific issue with SLES 12 (and I would assume the majority of the sma Ah, I misread the blurb in installation.adoc. Will update. http://gerrit.cloudera.org:8080/#/c/3500/1/docs/contributing.adoc File docs/contributing.adoc: Line 151: We are in the process of removing all remaining `boost` dependencies from the > We still have 'approved' boost libraries. I would vote to keep the approve I really was hoping to avoid introducing new Boost usage, so that we stand a chance of removing the whole thing some day. Is your point that adding a new usage of e.g. boost::optional isn't a big deal because, if/when we do replace it, updating an additional call-site is no big deal? -- To view, visit http://gerrit.cloudera.org:8080/3500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id5b73a8e2a86099429b6032023a01a0da7b02371 Gerrit-PatchSet: 1 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] locks: stop using errno around base::NumCPUs and base::MaxCPUIndex
Adar Dembo has submitted this change and it was merged. Change subject: locks: stop using errno around base::NumCPUs and base::MaxCPUIndex .. locks: stop using errno around base::NumCPUs and base::MaxCPUIndex It's not clear why we ever did this. InitializeSystemInfo() doesn't explicitly set errno itself, nor does it clear it in the event of an ignored error (e.g. ReadIntFromFile() on a file that doesn't exist). Without this fix, I consistently get the following when I run rwlock-perf: Test Threads Cycles -- WARNING: Logging before InitGoogleLogging() is written to STDERR F0625 12:20:24.620546 12138 locks.h:163] Check failed: (*__errno_location ()) == 0 (2 vs. 0) No such file or directory *** Check failure stack trace: *** Amusingly, I only see this in release builds because in a dynamically linked rwperf-test the very first call to InitializeSystemInfo() (subsequent calls are cached) takes place in a location that doesn't check errno upon return. Change-Id: I7c048a5a541bd3bfd54f8ac4acc1dd9e379c47ad Reviewed-on: http://gerrit.cloudera.org:8080/3495 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/consensus/log_index.cc M src/kudu/experiments/rwlock-perf.cc M src/kudu/rpc/negotiation.cc M src/kudu/util/locks.h 4 files changed, 13 insertions(+), 14 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I7c048a5a541bd3bfd54f8ac4acc1dd9e379c47ad 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
[kudu-CR] Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3511 to review the following change. Change subject: Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange .. Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange This test was supposed to wait until the master had committed a given operation, but in fact was only waiting until all of the servers had _replicated_ the operation. In TSAN builds where things ran slower, this caused it to be about 7% flaky. The fix is to simply wait for the op to be committed, not just replicated. With the fix, I looped it 100 times and it passed. Change-Id: Ibb5875e8d4200b0058cf7e7c4ee04771b91d2a24 --- M src/kudu/integration-tests/raft_consensus-itest.cc 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/11/3511/1 -- To view, visit http://gerrit.cloudera.org:8080/3511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibb5875e8d4200b0058cf7e7c4ee04771b91d2a24 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy
[kudu-CR] locks: add new read-write mutex
Adar Dembo has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/3496/2/src/kudu/util/rw_mutex.cc File src/kudu/util/rw_mutex.cc: Line 26: DCHECK_EQ(0, rv) << strerror(rv); > Yes it does. As do the unused 'rv' in Mutex and ConditionVariable. Quick note: these don't cause unused warnings, at least not with gcc 5.3. There was one warning in mutex.cc; I've fixed it. Line 34: pthread_rwlock_init(&native_handle_, NULL); > Does it make sense to check for return value here as well? The manpage says it should never return non-zero, but I've added the check anyway; it doesn't hurt and is more defensive. -- 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: 2 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: Yes
[kudu-CR] Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange
Kudu Jenkins has posted comments on this change. Change subject: Fix flakiness in RaftConsensusITest.TestMasterNotifiedOnConfigChange .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2051/ -- To view, visit http://gerrit.cloudera.org:8080/3511 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibb5875e8d4200b0058cf7e7c4ee04771b91d2a24 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) kudu flume sink blog post
Ara Ebrahimi has uploaded a new change for review. http://gerrit.cloudera.org:8080/3510 Change subject: kudu flume sink blog post .. kudu flume sink blog post Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed --- A _posts/2016-07-06-flume.md 1 file changed, 141 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/10/3510/1 -- To view, visit http://gerrit.cloudera.org:8080/3510 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I810146ab24c88bc6cc562d81746b9bf5303396ed Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Ara Ebrahimi
[kudu-CR] locks: add new read-write mutex
Alexey Serbin has posted comments on this change. Change subject: locks: add new read-write mutex .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/3496/2/src/kudu/util/rw_mutex.cc File src/kudu/util/rw_mutex.cc: Line 34: pthread_rwlock_init(&native_handle_, NULL); Does it make sense to check for return value here as well? -- 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: 2 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: Yes
[kudu-CR] Integrate the result tracker with writes
Kudu Jenkins has posted comments on this change. Change subject: Integrate the result tracker with writes .. Patch Set 7: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2050/ -- 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: 7 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: No
[kudu-CR] Add a test for the integration of RequestTracker with the client and ResultTracker with the server
Kudu Jenkins 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 6: -Verified Build Started http://104.196.14.100/job/kudu-gerrit/2048/ -- 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: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Add a test for the integration of RequestTracker with the client and ResultTracker with the server
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3505 to look at the new patch set (#6). Change subject: Add a test for the integration of RequestTracker with the client and ResultTracker with the server .. Add a test for the integration of RequestTracker with the client and ResultTracker with the server This adds a test for the integration of the RequestTracker in the client and the ResultTracker in the server. Specifically it tests that RetriableRpc works in combination with replayed results from the ResultTracker and that multiple simultaneous (similar) RPCs from the client are not executed more than once. Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b --- M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc-test-base.h M src/kudu/rpc/rtest.proto 4 files changed, 367 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/3505/6 -- To view, visit http://gerrit.cloudera.org:8080/3505 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic193dd5a547fe2a107a2ab1625f93fa0c19cc61b Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Integrate the request tracker with the client
Kudu Jenkins has posted comments on this change. Change subject: Integrate the request tracker with the client .. Patch Set 14: Build Started http://104.196.14.100/job/kudu-gerrit/2047/ -- To view, visit http://gerrit.cloudera.org:8080/3080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I94207c294452fcbdb3a7901fdb702674d47553ee Gerrit-PatchSet: 14 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves 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
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3192 to look at the new patch set (#16). Change subject: Integrate the ResultTracker into the rpc subsystem .. Integrate the ResultTracker into the rpc subsystem This integrates the ResultTracker into the rpc subsystem by allowing to specify a new method option when defining rpc service methods. When this method option is specificed _and_ the call's rpc header has a RequestIdPB the results for the call will be tracked and the call may have exactly once semantics. This allows to have the simplest form of exactly once semantics for single server calls. Of course limitations apply, like response persistence across restarts is not supported, neither is propagating/rebuilding responses to/on replicas for replicated calls. If support for this is needed it will have to be done ad-hoc, case by case. A follow up patch tests this in integration with the client. Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 --- M src/kudu/rpc/CMakeLists.txt M src/kudu/rpc/inbound_call.cc M src/kudu/rpc/protoc-gen-krpc.cc M src/kudu/rpc/rpc_context.cc M src/kudu/rpc/rpc_context.h M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/service_if.cc M src/kudu/rpc/service_if.h 8 files changed, 138 insertions(+), 42 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/3192/16 -- To view, visit http://gerrit.cloudera.org:8080/3192 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1d624810350feceefe244e0319e22cac241bf0d6 Gerrit-PatchSet: 16 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] Integrate the ResultTracker into the rpc subsystem
Kudu Jenkins has posted comments on this change. Change subject: Integrate the ResultTracker into the rpc subsystem .. Patch Set 16: Build Started http://104.196.14.100/job/kudu-gerrit/2049/ -- 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: 16 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] Avoid missing 'override' keyword warnings in raft consensus-test.cc
David Ribeiro Alves has submitted this change and it was merged. Change subject: Avoid missing 'override' keyword warnings in raft_consensus-test.cc .. Avoid missing 'override' keyword warnings in raft_consensus-test.cc In this test we're using gmock and the compiler spews out warnings that the mocked methods are missing the 'override' keyword. This just makes it so that we ignore that warning in that file. Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9 Reviewed-on: http://gerrit.cloudera.org:8080/3289 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/consensus/CMakeLists.txt 1 file changed, 8 insertions(+), 0 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3289 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I687a05ed560003721d4a05bd7a01261e03bd77d9 Gerrit-PatchSet: 4 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