[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7932 to look at the new patch set (#2). Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset .. [periodic-test] fix flaky PeriodicTimerTest.TestReset Fixed flake in PeriodicTimerTest.TestReset. The test was a little bit flaky because sometimes the main test thread might have slept for much longer than requested, sometimes longer than the period of the task. If that happened, the test failed with messages like the following: src/kudu/rpc/periodic-test.cc:116: Failure Expected: 0 To be equal to: counter_ Which is: 1 This fix adds code which short-circuits the test if the actual sleep time was too long. Prior to this fix, if running a DEBUG or ASAN build with --stress-cpu-threads=32, the was about 2 failures in every 1K run: http://dist-test.cloudera.org//job?job_id=aserbin.1504248200.5336 After the fix there are no failures anymore: http://dist-test.cloudera.org//job?job_id=aserbin.1504248606.11445 Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063 --- M src/kudu/rpc/periodic-test.cc 1 file changed, 17 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/7932/2 -- To view, visit http://gerrit.cloudera.org:8080/7932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] rpc: allow setting --rpc tls min protocol on older RHEL versions
Henry Robinson has posted comments on this change. Change subject: rpc: allow setting --rpc_tls_min_protocol on older RHEL versions .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7821/2/src/kudu/security/tls_context.cc File src/kudu/security/tls_context.cc: PS2, Line 71: 0x1000U Here is a fun thing I discovered: henry@hnr-optiplex:/data/henry/src/cloudera/impala-toolchain/openssl-1.0.0s/include (master) $ grep -r 0x1000L * openssl/ssl.h:# define SSL_OP_PKCS1_CHECK_2 0x1000L henry@hnr-optiplex:/data/henry/src/cloudera/impala-toolchain/openssl-1.0.0s/include (master) $ cd ../../openssl-1.0.2l/include/ henry@hnr-optiplex:/data/henry/src/cloudera/impala-toolchain/openssl-1.0.2l/include $ grep -r 0x1000L * openssl/ssl.h:# define SSL_OP_NO_TLSv1_1 0x1000L In OpenSSL 1.0.0, the constant that became SSL_OP_NO_TLSv1_1 in 1.0.1 was already in use for an esoteric option that messes with the cryptographic protocol for fault injection (deprecated in 1.0.1). I can't recall enough about your requirements to puzzle through whether this is a real problem for you, but theoretically it does mean that anyone linked against 1.0.0 that tries to set --rpc_tls_min_protocol=tlsv1.1 will get some unexpected behaviour. IIRC, Kudu isn't expected to work against 1.0.0 anyhow, so this may be an academic issue for you. In Impala, we're probably going to have to use SSLeay() to detect the OpenSSL version at runtime. -- To view, visit http://gerrit.cloudera.org:8080/7821 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic61f31788d63072fae609c6a2186e52d5e2467b7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] feat: add task interruption checking for RowIterator
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7753 to look at the new patch set (#3). Change subject: feat: add task interruption checking for RowIterator .. feat: add task interruption checking for RowIterator Once we the submit a spark job to spark cluster, we cannot cancel or stop the kudu-spark job immediately before checking task interruption, we can kill the job immediately by clicking the "kill" link on the spark Web UI. Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814 --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala 1 file changed, 5 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/7753/3 -- To view, visit http://gerrit.cloudera.org:8080/7753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: caiconghui Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: caiconghui
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Mike Percy has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 5: Code-Review+2 I manually rebased Adar's Patch Set 2 onto master and re-pushed it to Gerrit. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Hello Dan Burkert, Grant Henke, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7927 to look at the new patch set (#5). Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. gradle: convert gradle-wrapper.properties into a real generated file Let's handle gradle-wrapper.properties the same way as gradle-wrapper.jar: by downloading it during invocations of gradlew if it's not already there. I also put gradle/wrapper into .gitignore since it is only expected to contain generated files. Finally, I added an ASL2 header to gradlew and linked to it from Kudu's central license file. Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 --- M LICENSE.txt M java/.gitignore M java/gradle/wrapper.gradle D java/gradle/wrapper/gradle-wrapper.properties M java/gradlew 5 files changed, 31 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/5 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Mike Percy has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 4: > Sorry on phone it won't let me post inthe thread. I don't see why > the graders case needs to be explained further. It's no different > than the have files a line above. OK, your argument is convincing. I would be more comfortable if the included file(s) had copyright notices or something mentioning the Gradle project (in order to indicate provenance) but I suppose the fact that it's "gradlew" is clear enough where it came from and due to the ASL 2.0 license, attribution is not required. So +2 from me on Patch Set 2. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [periodic-test] fix flaky PeriodicTimerTest.TestReset
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/7932 Change subject: [periodic-test] fix flaky PeriodicTimerTest.TestReset .. [periodic-test] fix flaky PeriodicTimerTest.TestReset Fixed flake in PeriodicTimerTest.TestReset. The test was a little bit flaky because it might happen that the main test thread slept much more than requested 1 ms, sometimes longer than the period of the task. In such cases, the test failed with messages like the following: src/kudu/rpc/periodic-test.cc:116: Failure Expected: 0 To be equal to: counter_ Which is: 1 This fix adds the code which verifies that the actual sleep time of the main tests thread was not greater than the specified threshold (1/2 of task period). Also, the period of the task in non-TSAN modes has been bumped up to 50 ms. Prior to this fix if running the DEBUG build with --stress-cpu-threads=16, the was about 1 failure in every 1K run: http://dist-test.cloudera.org//job?job_id=aserbin.1504229055.4431 After the fix, there are no failures anymore: http://dist-test.cloudera.org//job?job_id=aserbin.1504228829.30928 Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063 --- M src/kudu/rpc/periodic-test.cc 1 file changed, 13 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/7932/1 -- To view, visit http://gerrit.cloudera.org:8080/7932 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6848bee0e830075aba3ae5d8ed49f99038043063 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Dan Burkert has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 2: Sorry on phone it won't let me post inthe thread. I don't see why the graders case needs to be explained further. It's no different than the have files a line above. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Mike Percy has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7927/2/LICENSE.txt File LICENSE.txt: Line 563: java/gradlew: Apache 2.0 license > 'this file'? Wouldn't that refer to LICENSE.txt in this context? I think it's clear we are referring to java/gradlew here, based on text further up in LICENSE.txt such as "Some portions of this module are derived from code from LevelDB", however we could call it a module or a script instead. http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew File java/gradlew: Line 3: # Copyright 2017 the original author or authors. > This is what a bunch of license headers look like in Gradle. We don't nece OK. Let's leave this as-is then. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 2: > It's possible because the remote can time out and retry based on > the leader telling it to retry, and those requests can be stuck in > the server's RPC queue. See KUDU-1436 for an early example of this. Hmm, and we don't want to pay the cost of instantiating any sessions beyond the first one? Okay, I guess I can buy that. I was asking because if we didn't mind creating the occasional extra session or two (and just let them get cleaned up via "expiration"), we could use an incrementing integer as the session key, which would guarantee no session reuse at all, thus avoiding the thorny synchronization around session creation. What do you think about using a per-session std::once to guarantee Init is called? The synchronization would be something like this: Take lock. Look for existing entry in session map. If not there, create a new session and it to the map. Release lock. Create a ScopedCleanup that removes the session with the lock held. RETURN_NOT_OK(Init) // via session->once, so it's a no-op if the session is already initted cleanup.cancel() This way we don't have to worry about lock interaction: the session lock and the once's spinlock are independent. -- To view, visit http://gerrit.cloudera.org:8080/7919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05f5b3c6a012067c95bb54f040bee2bb19388bfe Gerrit-PatchSet: 2 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: Tidy Bot Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Dan Burkert has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7927/2/LICENSE.txt File LICENSE.txt: Line 563: java/gradlew: Apache 2.0 license > I think we should add something like: 'this file'? Wouldn't that refer to LICENSE.txt in this context? http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew File java/gradlew: Line 3: # Copyright 2017 the original author or authors. > how about: the original author or authors from the Gradle project. This is what a bunch of license headers look like in Gradle. We don't necessarily know that the Gradle project holds copyright on this file. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Grant Henke has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 4: (2 comments) re: "real distribution" I mean from maven central or http://services.gradle.org/distributions/ instead of github. The problem is they don't publish the gradle-wrapper jar as a standalone artifact since this isn't the usual pattern. http://gerrit.cloudera.org:8080/#/c/7927/4/java/gradle/wrapper.gradle File java/gradle/wrapper.gradle: Line 29 Do we want to "override" the wrapper command to replace the gradlew with your script? That will ensure the gradle version in dependencies.gradle is still used. My goal in making "gradle wrapper" still work was not to break user expectations. Especially those familiar with gradle. http://gerrit.cloudera.org:8080/#/c/7927/4/java/gradlew File java/gradlew: Line 1: #!/bin/bash We should test that this works with IDE integrations like IntelliJ. They expect this script, but I am not sure if they have other expectations. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] [doc] add info about the iwyu target
Alexey Serbin has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 6: > Patch Set 5: Verified+1 Thanks. Yep, it's already known issue with kinit: java.io.IOException: process 'kinit' failed Todd posted a patch for fixing that. -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Mike Percy has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Mike Percy has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 2: > > Having such a session id design is more about de-duplicating > > begin-session requests than reusing existing sessions. Since it's > > possible for multiple requests from the same remote to queue up, > > due to timing issues that are difficult to control, we don't want > > to end up creating many duplicate sessions for the same remote, > > each of which is holding open many files and using up memory > until > > the sessions time out. > > How is it possible for there to be multiple requests from the same > remote? TSTabletManager::RunTabletCopy fails every attempt beyond > the first one. So there should be just one TabletCopyClient for > every peer/tablet ID pair. Also, TabletCopyClient::Start sends the > "begin session" RPC synchronously and doesn't appear to retry, so I > don't see how a single tablet copy client could send that RPC > multiple times either. > > Or is this about the remote failing, restarting, retrying the copy, > and reusing its existing session? It's possible because the remote can time out and retry based on the leader telling it to retry, and those requests can be stuck in the server's RPC queue. See KUDU-1436 for an early example of this. -- To view, visit http://gerrit.cloudera.org:8080/7919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05f5b3c6a012067c95bb54f040bee2bb19388bfe Gerrit-PatchSet: 2 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: Tidy Bot Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Hello Dan Burkert, Mike Percy, Grant Henke, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7927 to look at the new patch set (#4). Change subject: gradle: convert the rest of the gradle wrapper into generated files .. gradle: convert the rest of the gradle wrapper into generated files This patch replaces gradlew with a new script that downloads the real gradlew (as well as the other gradle wrapper content) from GitHub before invoking it. Thus we can treat the entirety of the wrapper (including gradlew itself) as generated files. Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 --- M java/.gitignore M java/gradle/wrapper.gradle D java/gradle/wrapper/gradle-wrapper.properties M java/gradlew 4 files changed, 44 insertions(+), 214 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/4 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [doc] add info about the iwyu target
Mike Percy has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [doc] add info about the iwyu target
Mike Percy has submitted this change and it was merged. Change subject: [doc] add info about the iwyu target .. [doc] add info about the iwyu target Added information about the cmake-generated target to run IWYU against the updated files. In addition, fixed the following error reported by asciidoctor: asciidoctor: ERROR: README.adoc: line 414: only book doctypes \ can contain level 0 sections Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Reviewed-on: http://gerrit.cloudera.org:8080/7928 Reviewed-by: Adar Dembo Reviewed-by: Mike Percy Tested-by: Mike Percy --- M README.adoc 1 file changed, 18 insertions(+), 2 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified Adar Dembo: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Mike Percy
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Adar Dembo has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7927/3/java/gradlew File java/gradlew: PS3, Line 40: # Run the real wrapper. By using 'source' we can trick the APP_HOME detection : # in gradlew to think that APP_HOME=$JAVA_DIR. That way it'll naturally find : # the gradle wrapper content in the gradle/wrapper subdirectory. > How about something like: Thanks. I used what you wrote modulo a few small changes. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Adar Dembo has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 3: > I think this is okay for this release since the gradle build is > experimental but we should use a real distribution link if possible > in the future. What do you mean by "real distribution link"? Can you elaborate? -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Adar Dembo has submitted this change and it was merged.
Change subject: master: always use smart pointers when accessing TableInfo and
TabletInfo
..
master: always use smart pointers when accessing TableInfo and TabletInfo
Mixing scoped_refptr and raw access is error-prone and complicated code by
forcing the use of both in certain places. It's fair to be concerned about
the overhead; as far as I can tell it's minimal, because it's either in
functions that are already "slow" (like {Create,Alter,Delete}Table and the
equivalent in CatalogManagerBgTasks), or the call sites pass the object by
cref and thus avoid an extra ref/deref in the first place.
The one exception is in TableInfo::tablet_map, which would create a cycle if
it held strong refs to its TabletInfos. I tried to prevent this exception
from leaking outside of TableInfo.
I also reduced the number of ways to add/remove TabletInfo objects to a
TableInfo down to one.
Finally, I couldn't help myself and did some more modernization:
- Removed a couple std:: prefixes.
- Added "auto" to some loops.
- Replaced some vector::push_back calls with emplace_back.
- Reformatted double '>' (C++11 allows them to be adjacent).
Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
Reviewed-on: http://gerrit.cloudera.org:8080/7909
Tested-by: Adar Dembo
Reviewed-by: Alexey Serbin
---
M src/kudu/master/catalog_manager-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
6 files changed, 191 insertions(+), 251 deletions(-)
Approvals:
Adar Dembo: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/7909
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
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: Tidy Bot
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Adar Dembo has posted comments on this change. Change subject: master: always use smart pointers when accessing TableInfo and TabletInfo .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7909/3/src/kudu/master/sys_catalog.h File src/kudu/master/sys_catalog.h: Line 243: void ReqAddTable(tserver::WriteRequestPB* req, > Looks like these could take TableInfo by cref. As we discussed, I'd prefer to keep it the way it is for consistency with the other callers. -- To view, visit http://gerrit.cloudera.org:8080/7909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae 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: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Mike Percy has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 3: Code-Review+1 (1 comment) looks good, just a comment nit http://gerrit.cloudera.org:8080/#/c/7927/3/java/gradlew File java/gradlew: PS3, Line 40: # Run the real wrapper. By using 'source' we can trick the APP_HOME detection : # in gradlew to think that APP_HOME=$JAVA_DIR. That way it'll naturally find : # the gradle wrapper content in the gradle/wrapper subdirectory. How about something like: Run the "real" gradlew script. Even though we downloaded it into the wrapper/ directory, if we invoke it via 'source' it will be tricked into thinking it is running from the java root directory, which is where it expects to live. That way it'll naturally find the gradle wrapper content in the gradle/wrapper subdirectory. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Grant Henke has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 3: Code-Review+1 I think this is okay for this release since the gradle build is experimental but we should use a real distribution link if possible in the future. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Dan Burkert has posted comments on this change. Change subject: gradle: convert the rest of the gradle wrapper into generated files .. Patch Set 3: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [doc] add info about the iwyu target
Mike Percy has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [doc] add info about the iwyu target
Adar Dembo has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert the rest of the gradle wrapper into generated files
Hello Dan Burkert, Grant Henke, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7927 to look at the new patch set (#3). Change subject: gradle: convert the rest of the gradle wrapper into generated files .. gradle: convert the rest of the gradle wrapper into generated files This patch replaces gradlew with a new script that downloads the real gradlew (as well as the other gradle wrapper content) from GitHub before invoking it. Thus we can treat the entirety of the wrapper (including gradlew itself) as generated files. Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 --- M java/.gitignore M java/gradle/wrapper.gradle D java/gradle/wrapper/gradle-wrapper.properties M java/gradlew 4 files changed, 41 insertions(+), 214 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/3 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Dan Burkert has posted comments on this change. Change subject: master: always use smart pointers when accessing TableInfo and TabletInfo .. Patch Set 3: (1 comment) A bunch of the s/push_back/emplace_back calls aren't really doing anything different because they are operating on crefs. It's not wrong, but I don't think it's necessarily better to use emplace_back. I don't feel strongly about it. http://gerrit.cloudera.org:8080/#/c/7909/3/src/kudu/master/sys_catalog.h File src/kudu/master/sys_catalog.h: Line 243: void ReqAddTable(tserver::WriteRequestPB* req, Looks like these could take TableInfo by cref. -- To view, visit http://gerrit.cloudera.org:8080/7909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae 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: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] [doc] add info about the iwyu target
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7928 to look at the new patch set (#5). Change subject: [doc] add info about the iwyu target .. [doc] add info about the iwyu target Added information about the cmake-generated target to run IWYU against the updated files. In addition, fixed the following error reported by asciidoctor: asciidoctor: ERROR: README.adoc: line 414: only book doctypes \ can contain level 0 sections Change-Id: I87da493486500cde0cd226614f8a19985d295a96 --- M README.adoc 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/7928/5 -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [doc] add info about the iwyu target
Alexey Serbin has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7928/2/README.adoc File README.adoc: PS2, Line 264: > missing word: "the" Done PS2, Line 267: w > nit: capitalize Why because it's the title of an article Done -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] [doc] add info about the iwyu target
Alexey Serbin has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 4: > (2 comments) > > Sorry, you will need to manually rebase after I merged my "make > tidy" patch That's fine -- I did that. -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [doc] add info about the iwyu target
Hello Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7928 to look at the new patch set (#4). Change subject: [doc] add info about the iwyu target .. [doc] add info about the iwyu target Added information about the cmake-generated target to run IWYU against the updated files. In addition, fixed the following error reported by asciidoctor: asciidoctor: ERROR: README.adoc: line 414: only book doctypes \ can contain level 0 sections Change-Id: I87da493486500cde0cd226614f8a19985d295a96 --- M README.adoc 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/7928/4 -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] [doc] add info about the iwyu target
Mike Percy has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 3: Code-Review+2 I thought you had to manually rebase but my previous nits weren't very important, so LGTM -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [doc] add info about the iwyu target
Mike Percy has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 3: Oh, what do you know, automatic rebase worked... -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [doc] add info about the iwyu target
Mike Percy has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 2: (2 comments) Sorry, you will need to manually rebase after I merged my "make tidy" patch http://gerrit.cloudera.org:8080/#/c/7928/2/README.adoc File README.adoc: PS2, Line 264: missing word: "the" PS2, Line 267: w nit: capitalize Why because it's the title of an article -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Add "make tidy" target
Mike Percy has submitted this change and it was merged. Change subject: Add "make tidy" target .. Add "make tidy" target This adds a simple cmake target to run a local clang-tidy check, instead of having to come up with the incantation to run the tidy script. Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 Reviewed-on: http://gerrit.cloudera.org:8080/7917 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M CMakeLists.txt M README.adoc 2 files changed, 21 insertions(+), 0 deletions(-) Approvals: Alexey Serbin: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Alexey Serbin has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew File java/gradlew: PS2, Line 86: https://raw.githubusercontent.com/gradle/gradle/v4.1.0/gradle/wrapper/gradle-wrapper.properties nit: what if they put a redirect there in future? Consider adding '-L' option while invoking the curl utility. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 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: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] [doc] add info about the iwyu target
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7928 to look at the new patch set (#2). Change subject: [doc] add info about the iwyu target .. [doc] add info about the iwyu target Added information about the cmake-generated target to run IWYU against the updated files. In addition, fixed the following error reported by asciidoctor: asciidoctor: ERROR: README.adoc: line 414: only book doctypes \ can contain level 0 sections Change-Id: I87da493486500cde0cd226614f8a19985d295a96 --- M README.adoc 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/7928/2 -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [doc] add info about the iwyu target
Alexey Serbin has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/7928/1/README.adoc File README.adoc: PS1, Line 267: consistent > Maybe put something else here? The rendered view says "For more information Done PS1, Line 269: You can run the IWYU checks via the cmake-generated target `iwyu`. E.g., : once generated makefiles for make, you can run the IWYU verification executing > How about copying the make ilint instructions and saying "You can run the I Done PS1, Line 277: This will scan all relevant dirty files in your working tree, and files : changed since the last committed upstream changelist. > Likewise, copy the verbiage from make ilint. Done -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Mike Percy has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 2: -Code-Review -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Mike Percy has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7927/2/LICENSE.txt File LICENSE.txt: Line 563: java/gradlew: Apache 2.0 license I think we should add something like: This file is derived from code in the Gradle project, copyright (c) the original author(s) and licensed under the Apache 2.0 License. http://gerrit.cloudera.org:8080/#/c/7927/2/java/gradlew File java/gradlew: Line 3: # Copyright 2017 the original author or authors. how about: the original author or authors from the Gradle project. -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Mike Percy has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Hello Dan Burkert, Grant Henke, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7927 to look at the new patch set (#2). Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. gradle: convert gradle-wrapper.properties into a real generated file Let's handle gradle-wrapper.properties the same way as gradle-wrapper.jar: by downloading it during invocations of gradlew if it's not already there. I also put gradle/wrapper into .gitignore since it is only expected to contain generated files. Finally, I added an ASL2 header to gradlew and linked to it from Kudu's central license file. Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 --- M LICENSE.txt M java/.gitignore M java/gradle/wrapper.gradle D java/gradle/wrapper/gradle-wrapper.properties M java/gradlew 5 files changed, 31 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/2 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Grant Henke has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Adar Dembo has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7927/1/java/gradlew File java/gradlew: Line 3: ## > What is the license of this file? ASL presumably, since that's the license for all of Gradle. The upstream version of gradlew (https://github.com/gradle/gradle/blob/master/gradlew) doesn't have a license header either, and this is a copy of it (modulo the curl bits that we "inject"). -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] [doc] add info about the iwyu target
Adar Dembo has posted comments on this change. Change subject: [doc] add info about the iwyu target .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/7928/1/README.adoc File README.adoc: PS1, Line 267: consistent Maybe put something else here? The rendered view says "For more information on what consistent means, see consistent." PS1, Line 269: You can run the IWYU checks via the cmake-generated target `iwyu`. E.g., : once generated makefiles for make, you can run the IWYU verification executing How about copying the make ilint instructions and saying "You can run the IWYU checks via cmake using the `iwyu` target:" PS1, Line 277: This will scan all relevant dirty files in your working tree, and files : changed since the last committed upstream changelist. Likewise, copy the verbiage from make ilint. -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Alexey Serbin has posted comments on this change. Change subject: master: always use smart pointers when accessing TableInfo and TabletInfo .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae 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: Tidy Bot Gerrit-HasComments: No
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Adar Dembo has posted comments on this change. Change subject: master: always use smart pointers when accessing TableInfo and TabletInfo .. Patch Set 3: Verified+1 There were two TSAN failures. One was KUDU-2059. The other was a failure in Test_KUDU_1735: /home/jenkins-slave/workspace/kudu-master/0/src/kudu/integration-tests/raft_consensus-itest.cc:2947: Failure Failed Bad status: Illegal state: Leader has not yet committed an operation in its own term I don't think either failure is related to this patch. -- To view, visit http://gerrit.cloudera.org:8080/7909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae 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: Tidy Bot Gerrit-HasComments: No
[kudu-CR] [doc] add info about the iwyu target
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/7928 Change subject: [doc] add info about the iwyu target .. [doc] add info about the iwyu target Added information about the cmake-generated target to run IWYU against the updated files. In addition, fixed the following error reported by asciidoctor: asciidoctor: ERROR: README.adoc: line 414: only book doctypes \ can contain level 0 sections Change-Id: I87da493486500cde0cd226614f8a19985d295a96 --- M README.adoc 1 file changed, 19 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/7928/1 -- To view, visit http://gerrit.cloudera.org:8080/7928 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I87da493486500cde0cd226614f8a19985d295a96 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Mike Percy has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7927/1/java/gradlew File java/gradlew: Line 3: ## What is the license of this file? -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Dan Burkert has posted comments on this change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] gradle: convert gradle-wrapper.properties into a real generated file
Hello Dan Burkert, Grant Henke, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7927 to review the following change. Change subject: gradle: convert gradle-wrapper.properties into a real generated file .. gradle: convert gradle-wrapper.properties into a real generated file Let's handle gradle-wrapper.properties the same way as gradle-wrapper.jar: by downloading it during invocations of gradlew if it's not already there. I also put gradle/wrapper into .gitignore since it is only expected to contain generated files. Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 --- M java/.gitignore M java/gradle/wrapper.gradle D java/gradle/wrapper/gradle-wrapper.properties M java/gradlew 4 files changed, 16 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/7927/1 -- To view, visit http://gerrit.cloudera.org:8080/7927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I02f31865b27f186291d18f7683c5e40698b8ce37 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke
[kudu-CR] Fix RAT warnings
Grant Henke has posted comments on this change. Change subject: Fix RAT warnings .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7925/3/java/gradle/wrapper/gradle-wrapper.properties File java/gradle/wrapper/gradle-wrapper.properties: Line 1 Per our chats, this is a generate file. I think Adar is fixing this. -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Fix RAT complaints in gradle properties files
Mike Percy has abandoned this change. Change subject: Fix RAT complaints in gradle properties files .. Abandoned Dan has an uber patch that he is rolling all of these changes into -- To view, visit http://gerrit.cloudera.org:8080/7926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I6fea0ce4f6dae70fab45c114b0cfeda35b9a8798 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Fix RAT warnings
Mike Percy has posted comments on this change. Change subject: Fix RAT warnings .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7925/1/build-support/release/rat_exclude_files.txt File build-support/release/rat_exclude_files.txt: Line 30: python/MANIFEST.in > we may need to mention this in LICENSE.txt actually since this is from AsyncHBase I think we are already covered in license.txt -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Fix RAT warnings
Mike Percy has posted comments on this change. Change subject: Fix RAT warnings .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/7925/3/java/gradle/wrapper/gradle-wrapper.properties File java/gradle/wrapper/gradle-wrapper.properties: Line 1 deleted line -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Fix RAT warnings
Mike Percy has posted comments on this change. Change subject: Fix RAT warnings .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7925/2/build-support/release/rat_exclude_files.txt File build-support/release/rat_exclude_files.txt: Line 11: build-support/iwyu/iwyu_tool.py This still needs a mention in LICENSE.txt http://gerrit.cloudera.org:8080/#/c/7925/2/java/gradle/wrapper/gradle-wrapper.properties File java/gradle/wrapper/gradle-wrapper.properties: PS2, Line 16: distributionBase=GRADLE_USER_HOME typo -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Fix RAT warnings
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7925 to look at the new patch set (#3). Change subject: Fix RAT warnings .. Fix RAT warnings Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe --- M build-support/release/rat_exclude_files.txt M java/gradle.properties M java/gradle/wrapper/gradle-wrapper.properties 3 files changed, 8 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/7925/3 -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] Fix RAT warnings
Mike Percy has posted comments on this change. Change subject: Fix RAT warnings .. Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/7925/1/build-support/release/rat_exclude_files.txt File build-support/release/rat_exclude_files.txt: Line 11: build-support/iwyu/iwyu_tool.py may need to mention this in LICENSE.txt Line 16: java/gradle.properties Can remove this w/ the patch I posted Line 17: java/gradle/wrapper/gradle-wrapper.properties Same Line 30: java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java we may need to mention this in LICENSE.txt Line 175: www/bootstrap/css/bootstrap.min.css cool, this stuff is already there in LICENSE.txt -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Fix RAT warnings
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7925 to look at the new patch set (#2). Change subject: Fix RAT warnings .. Fix RAT warnings Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe --- M build-support/release/rat_exclude_files.txt M java/gradle.properties M java/gradle/wrapper/gradle-wrapper.properties 3 files changed, 24 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/7925/2 -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] Fix RAT complaints in gradle properties files
Hello Dan Burkert, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7926 to review the following change. Change subject: Fix RAT complaints in gradle properties files .. Fix RAT complaints in gradle properties files RAT is complaining that these files don't have valid license headers. Change-Id: I6fea0ce4f6dae70fab45c114b0cfeda35b9a8798 --- M java/gradle.properties M java/gradle/wrapper/gradle-wrapper.properties 2 files changed, 18 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/7926/1 -- To view, visit http://gerrit.cloudera.org:8080/7926 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6fea0ce4f6dae70fab45c114b0cfeda35b9a8798 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Dan Burkert
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Adar Dembo has posted comments on this change. Change subject: master: always use smart pointers when accessing TableInfo and TabletInfo .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/7909/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: PS2, Line 1788: map> > nit: would TabletInfo::TabletInfoMap be a better choice here? Done PS2, Line 1789: map> > ditto Done PS2, Line 3683: .get() > nit: it seems this might be dropped Done PS2, Line 3684: .get() > ditto Done -- To view, visit http://gerrit.cloudera.org:8080/7909 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae 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: Tidy Bot Gerrit-HasComments: Yes
[kudu-CR] master: always use smart pointers when accessing TableInfo and TabletInfo
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7909
to look at the new patch set (#3).
Change subject: master: always use smart pointers when accessing TableInfo and
TabletInfo
..
master: always use smart pointers when accessing TableInfo and TabletInfo
Mixing scoped_refptr and raw access is error-prone and complicated code by
forcing the use of both in certain places. It's fair to be concerned about
the overhead; as far as I can tell it's minimal, because it's either in
functions that are already "slow" (like {Create,Alter,Delete}Table and the
equivalent in CatalogManagerBgTasks), or the call sites pass the object by
cref and thus avoid an extra ref/deref in the first place.
The one exception is in TableInfo::tablet_map, which would create a cycle if
it held strong refs to its TabletInfos. I tried to prevent this exception
from leaking outside of TableInfo.
I also reduced the number of ways to add/remove TabletInfo objects to a
TableInfo down to one.
Finally, I couldn't help myself and did some more modernization:
- Removed a couple std:: prefixes.
- Added "auto" to some loops.
- Replaced some vector::push_back calls with emplace_back.
- Reformatted double '>' (C++11 allows them to be adjacent).
Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
---
M src/kudu/master/catalog_manager-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
6 files changed, 191 insertions(+), 251 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/7909/3
--
To view, visit http://gerrit.cloudera.org:8080/7909
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3c74f2aa048ee85cc3a5f863ce8f38147e78c5ae
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 2: > Having such a session id design is more about de-duplicating > begin-session requests than reusing existing sessions. Since it's > possible for multiple requests from the same remote to queue up, > due to timing issues that are difficult to control, we don't want > to end up creating many duplicate sessions for the same remote, > each of which is holding open many files and using up memory until > the sessions time out. How is it possible for there to be multiple requests from the same remote? TSTabletManager::RunTabletCopy fails every attempt beyond the first one. So there should be just one TabletCopyClient for every peer/tablet ID pair. Also, TabletCopyClient::Start sends the "begin session" RPC synchronously and doesn't appear to retry, so I don't see how a single tablet copy client could send that RPC multiple times either. Or is this about the remote failing, restarting, retrying the copy, and reusing its existing session? -- To view, visit http://gerrit.cloudera.org:8080/7919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05f5b3c6a012067c95bb54f040bee2bb19388bfe Gerrit-PatchSet: 2 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: Tidy Bot Gerrit-HasComments: No
[kudu-CR] KUDU-2098. Drop Spark 1 Support
Dan Burkert has posted comments on this change.
Change subject: KUDU-2098. Drop Spark 1 Support
..
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/7690/2//COMMIT_MSG
Commit Message:
Line 9: Spark 2 has been available for over a year and the value of
May want to throw a link to
https://lists.apache.org/thread.html/1bcd0e048e7fe8d91f69c468d1577987215484395dd0540b7fa902cf@%3Cdev.kudu.apache.org%3E
in here.
http://gerrit.cloudera.org:8080/#/c/7690/2/java/README.md
File java/README.md:
Line 54: $ mvn verify
Probably should call out that this requires Java 8, since it's 7 or 8 above.
http://gerrit.cloudera.org:8080/#/c/7690/2/java/kudu-spark-tools/pom.xml
File java/kudu-spark-tools/pom.xml:
Line 30:
kudu-${spark.version.label}-tools_${scala.binary.version}
I think we should hard code this now, IntelliJ has a big problem with
properties in artifact names.
http://gerrit.cloudera.org:8080/#/c/7690/2/java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala
File
java/kudu-spark-tools/src/test/scala/org/apache/kudu/spark/tools/ITBigLinkedListTest.scala:
Line 41: ss)
keep the indent the same.
http://gerrit.cloudera.org:8080/#/c/7690/2/java/kudu-spark/pom.xml
File java/kudu-spark/pom.xml:
Line 23:
kudu-${spark.version.label}_${scala.binary.version}
Same here. I wouldn't be against getting rid of all of the interpolation of
spark and scala versions in artifact IDs, but maybe we'll need it again?
--
To view, visit http://gerrit.cloudera.org:8080/7690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5481cc0477f4d23d89b68ef510a6c9a2aa187537
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Add "make tidy" target
Alexey Serbin has posted comments on this change. Change subject: Add "make tidy" target .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add "make tidy" target
Alexey Serbin has posted comments on this change. Change subject: Add "make tidy" target .. Patch Set 2: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc File README.adoc: Line 263: === Running clang-tidy checks > I am feeling generous, but not that generous right now, so let's do that in SGTM -- I will send a patch for that shortly :) -- To view, visit http://gerrit.cloudera.org:8080/7917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Add "make tidy" target
Mike Percy has posted comments on this change.
Change subject: Add "make tidy" target
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7917/2/CMakeLists.txt
File CMakeLists.txt:
Line 1117: add_custom_target(tidy ${BUILD_SUPPORT_DIR}/clang_tidy_gerrit.py
-n HEAD)
> Would be nice to use get-upstream-commit.sh to figure out the exact set of
I agree it would be nice, but this is better than what we have today, and I
have more pressing tasks at the moment.
http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc
File README.adoc:
Line 263: === Running clang-tidy checks
> +1
I am feeling generous, but not that generous right now, so let's do that in a
follow up commit. ;)
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Mike Percy has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 2: > I'd like to better understand the concurrency model here. > > The tablet copy service maintains a session map whose key is > requestor ID + tablet ID. The requestor ID is just the UUID of the > tserver sending the "Begin Session" request. > > Under what circumstances can there be a "collision"? Seems like in > order to begin a copy the requestor will have already "locked" the > tablet (via TSTabletManager::StartTabletStateTransitionUnlocked), > so it won't initiate a second copy if one is in flight. And other > "Begin Session" requests from other tservers will have different > requestor IDs, so they won't collide. > > Much of the complexity (and this fix) have to do with the desired > behavior in the event of session ID reuse, so I want to understand > the circumstances and/or motivation behind reuse first. Having such a session id design is more about de-duplicating begin-session requests than reusing existing sessions. Since it's possible for multiple requests from the same remote to queue up, due to timing issues that are difficult to control, we don't want to end up creating many duplicate sessions for the same remote, each of which is holding open many files and using up memory until the sessions time out. -- To view, visit http://gerrit.cloudera.org:8080/7919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05f5b3c6a012067c95bb54f040bee2bb19388bfe Gerrit-PatchSet: 2 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: Tidy Bot Gerrit-HasComments: No
[kudu-CR] feat: add the wrapper for the Iterator by using InterruptibleIterator
Dan Burkert has posted comments on this change. Change subject: feat: add the wrapper for the Iterator by using InterruptibleIterator .. Patch Set 2: My only hesitation with TaskKilledException is that it's a developer API, so it's subject to change. I also can't find an API to actually do cancellation, so skipping the test is fine. -- To view, visit http://gerrit.cloudera.org:8080/7753 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0b4284f2c0a40cd7ba8cf2b76e0403592552c814 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: caiconghui Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: caiconghui Gerrit-HasComments: No
[kudu-CR] Fix RAT warnings
Hello Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/7925 to review the following change. Change subject: Fix RAT warnings .. Fix RAT warnings Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe --- M build-support/release/rat_exclude_files.txt M java/gradle.properties 2 files changed, 9 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/25/7925/1 -- To view, visit http://gerrit.cloudera.org:8080/7925 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0260ce9ce08790d3c6004dcdb0a78561691b5cbe Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7919 to look at the new patch set (#2). Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. KUDU-2124. Don't hold session lock while initializing a TabletCopySession This patch incorporates a lock table to hold reservations for sessions currently being initialized, instead of holding the session lock during session initialization. Implemented a regression test that injects latency into BeginTabletCopySession() calls. Change-Id: I05f5b3c6a012067c95bb54f040bee2bb19388bfe --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_copy-itest.cc M src/kudu/tserver/tablet_copy_service-test.cc M src/kudu/tserver/tablet_copy_service.cc M src/kudu/tserver/tablet_copy_service.h M src/kudu/tserver/tablet_copy_source_session.cc M src/kudu/tserver/tablet_server-test-base.h M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/tablet_server_test_util.cc M src/kudu/tserver/tablet_server_test_util.h 11 files changed, 245 insertions(+), 46 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/7919/2 -- To view, visit http://gerrit.cloudera.org:8080/7919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I05f5b3c6a012067c95bb54f040bee2bb19388bfe Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2055: Coalesce hole punching when deleting groups of blocks
Hao Hao has posted comments on this change. Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks .. Patch Set 3: > I'd like to avoid the mega-review that we had for the other patch > by carving this one up. Here are some ideas for subpatches: > 1. Patch to add BlockDeletionTransaction and plumb it through the > LBM. At this stage it's just a new abstraction and doesn't yet > change semantics or improve performance. > 2. Patch to introduce (and test) coalescing of intervals, in fairly > generic fashion. > 3. Patch to add coalescing support to BlockDeletionTransaction. If > this is tiny, maybe combine it with patch #2. > 4. Patch to start using BlockDeletionTransaction outside of the > 'fs' module. Sounds good, will do! -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] KUDU-2055: Coalesce hole punching when deleting groups of blocks
Adar Dembo has posted comments on this change. Change subject: KUDU-2055: Coalesce hole punching when deleting groups of blocks .. Patch Set 3: I'd like to avoid the mega-review that we had for the other patch by carving this one up. Here are some ideas for subpatches: 1. Patch to add BlockDeletionTransaction and plumb it through the LBM. At this stage it's just a new abstraction and doesn't yet change semantics or improve performance. 2. Patch to introduce (and test) coalescing of intervals, in fairly generic fashion. 3. Patch to add coalescing support to BlockDeletionTransaction. If this is tiny, maybe combine it with patch #2. 4. Patch to start using BlockDeletionTransaction outside of the 'fs' module. -- To view, visit http://gerrit.cloudera.org:8080/7656 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iecb252b3a5665d5471bb82301d0c8012a68de959 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: No
[kudu-CR] Add "make tidy" target
Alexey Serbin has posted comments on this change. Change subject: Add "make tidy" target .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/7917/2/CMakeLists.txt File CMakeLists.txt: Line 1118: add_dependencies(tidy pb-gen krpc-gen) > Why does tidy depend on these? I think it's necessary to have those stubs and proxies generated because otherwise tidy could report on that. E.g., if one those files are not present, tidy reports something like: INFO:root:/Users/aserbin/Projects/kudu/build-support/../src/kudu/client/client.h:30:10: error: 'kudu/client/client.pb.h' file not found [clang-diagnostic-error] #include "kudu/client/client.pb.h" Not sure whether those auto-generated files are needed to the analysis of the related entities in the code, though. http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc File README.adoc: Line 263: === Running clang-tidy checks > If you're feeling generous you could also doc the iwyu target. +1 It would be nice to include that here as well. :) -- To view, visit http://gerrit.cloudera.org:8080/7917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] KUDU-2124. Don't hold session lock while initializing a TabletCopySession
Adar Dembo has posted comments on this change. Change subject: KUDU-2124. Don't hold session lock while initializing a TabletCopySession .. Patch Set 1: I'd like to better understand the concurrency model here. The tablet copy service maintains a session map whose key is requestor ID + tablet ID. The requestor ID is just the UUID of the tserver sending the "Begin Session" request. Under what circumstances can there be a "collision"? Seems like in order to begin a copy the requestor will have already "locked" the tablet (via TSTabletManager::StartTabletStateTransitionUnlocked), so it won't initiate a second copy if one is in flight. And other "Begin Session" requests from other tservers will have different requestor IDs, so they won't collide. Much of the complexity (and this fix) have to do with the desired behavior in the event of session ID reuse, so I want to understand the circumstances and/or motivation behind reuse first. -- To view, visit http://gerrit.cloudera.org:8080/7919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I05f5b3c6a012067c95bb54f040bee2bb19388bfe 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: Tidy Bot Gerrit-HasComments: No
[kudu-CR] Implement a lock table
Mike Percy has posted comments on this change. Change subject: Implement a lock table .. Patch Set 2: Verified+1 Overriding unrelated flaky test failure of RaftConsensusITest.TestReplicaBehaviorViaRPC -- To view, visit http://gerrit.cloudera.org:8080/7918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0404478675e079ba147fbdb4b0bac9db0cdc87e5 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Add "make tidy" target
Adar Dembo has posted comments on this change.
Change subject: Add "make tidy" target
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/7917/2/CMakeLists.txt
File CMakeLists.txt:
Line 1117: add_custom_target(tidy ${BUILD_SUPPORT_DIR}/clang_tidy_gerrit.py
-n HEAD)
Would be nice to use get-upstream-commit.sh to figure out the exact set of
commits for clang-tidy to review. Then it'll have the same semantics as ilint
and iwyu.
Line 1118: add_dependencies(tidy pb-gen krpc-gen)
Why does tidy depend on these?
http://gerrit.cloudera.org:8080/#/c/7917/2/README.adoc
File README.adoc:
Line 263: === Running clang-tidy checks
If you're feeling generous you could also doc the iwyu target.
--
To view, visit http://gerrit.cloudera.org:8080/7917
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR](branch-1.5.x) docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has uploaded a new change for review. http://gerrit.cloudera.org:8080/7924 Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Reviewed-on: http://gerrit.cloudera.org:8080/7914 Reviewed-by: Dan Burkert Tested-by: Dan Burkert (cherry picked from commit 463d59ce79b5f7d69e9e80b87c90df5ed68a4270) --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 60 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/7924/1 -- To view, visit http://gerrit.cloudera.org:8080/7924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert
[kudu-CR](branch-1.5.x) docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has submitted this change and it was merged. Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Reviewed-on: http://gerrit.cloudera.org:8080/7914 Reviewed-by: Dan Burkert Tested-by: Dan Burkert (cherry picked from commit 463d59ce79b5f7d69e9e80b87c90df5ed68a4270) Reviewed-on: http://gerrit.cloudera.org:8080/7924 --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 60 insertions(+), 35 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/7924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-1.5.x) docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 1: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/7924 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Dan Burkert Gerrit-HasComments: No
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has submitted this change and it was merged. Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Reviewed-on: http://gerrit.cloudera.org:8080/7914 Reviewed-by: Dan Burkert Tested-by: Dan Burkert --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 60 insertions(+), 35 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 8: Code-Review+2 Carrying over Alexey and Adar's +2 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Hello Hao Hao, Andrew Wong, Adar Dembo, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7914 to look at the new patch set (#8). Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 60 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/7914/8 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/7914/6/docs/release_notes.adoc File docs/release_notes.adoc: PS6, Line 48: accepted > nit: accepted by default? Done PS6, Line 140: upper case > nit: single word Done -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Adar Dembo has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Andrew Wong has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/7914/6/docs/release_notes.adoc File docs/release_notes.adoc: PS6, Line 48: accepted nit: accepted by default? PS6, Line 140: upper case nit: single word -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/7914/6/docs/release_notes.adoc File docs/release_notes.adoc: PS6, Line 82: 'tablet move tool' > nit: 'tablet move' tool. Done -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Hello Hao Hao, Andrew Wong, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7914 to look at the new patch set (#7). Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 59 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/7914/7 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add "make tidy" target
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7917 to look at the new patch set (#2). Change subject: Add "make tidy" target .. Add "make tidy" target This adds a simple cmake target to run a local clang-tidy check, instead of having to come up with the incantation to run the tidy script. Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 --- M CMakeLists.txt M README.adoc 2 files changed, 21 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/17/7917/2 -- To view, visit http://gerrit.cloudera.org:8080/7917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Add "make tidy" target
Alexey Serbin has posted comments on this change. Change subject: Add "make tidy" target .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/7917 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77ab0376042b7ac754a24ebc7e18bcd6dc240011 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Hao Hao has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 6: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/7914/6/docs/release_notes.adoc File docs/release_notes.adoc: PS6, Line 82: 'tablet move tool' nit: 'tablet move' tool. -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Implement a lock table
Dan Burkert has posted comments on this change. Change subject: Implement a lock table .. Patch Set 2: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/7918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0404478675e079ba147fbdb4b0bac9db0cdc87e5 Gerrit-PatchSet: 2 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: No
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Alexey Serbin has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 6: Code-Review+2 (1 comment) LGTM. You might want to get more feedback from Adar and other guys who already reviewed the draft. http://gerrit.cloudera.org:8080/#/c/7914/5/docs/release_notes.adoc File docs/release_notes.adoc: Line 136: details. > I added a note about 2085. As far as I know the rest have never been seen Yep, that makes sense. -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Dan Burkert has posted comments on this change. Change subject: docs: light editing on 1.5 release notes; spark security docs .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/7914/5/docs/release_notes.adoc File docs/release_notes.adoc: PS5, Line 76: kudu` command line tool > Does it make sense to mention about the new 'pbc edit' command? I think we should skip that one, since it's very low level and dangerous. Line 124: > Maybe, it's worth mentioning: Done Line 136: > What about adding the following: I added a note about 2085. As far as I know the rest have never been seen in the wild, so in my opinion they don't meet the bar of noteworthiness. -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] docs: light editing on 1.5 release notes; spark security docs
Hello Andrew Wong, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7914 to look at the new patch set (#6). Change subject: docs: light editing on 1.5 release notes; spark security docs .. docs: light editing on 1.5 release notes; spark security docs Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f --- M docs/developing.adoc M docs/release_notes.adoc 2 files changed, 59 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/7914/6 -- To view, visit http://gerrit.cloudera.org:8080/7914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I49014dda6fcec328b3adf3a414ee334ab411e94f Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins
[kudu-CR](branch-1.5.x) KUDU-2078: Sink failure if batch size > session's flush buffer size
Mike Percy has submitted this change and it was merged. Change subject: KUDU-2078: Sink failure if batch size > session's flush buffer size .. KUDU-2078: Sink failure if batch size > session's flush buffer size The Flume sink uses manual flush mode, so if users set the sink's batch size parameter above the manual flush default buffer size, the sink could fail batches (over and over). This patch sets the session's buffer size (which is in terms of number of ops) to the same as the batch size, so this problem can no longer occur. I considered using AUTO_FLUSH_BACKGROUND for the flushing as well, but it can result in out-of-order writes, which might be unexpected semantics for Flume (as opposed to, say, Spark). Using AUTO_FLUSH_BACKGROUND with a high batch size would likely be more performant, but we can add that as an additional configuration later if the need arises. Change-Id: Id1c54bcecc3e13ae64dd90efe6cf53021517dcdf Reviewed-on: http://gerrit.cloudera.org:8080/7641 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy Reviewed-on: http://gerrit.cloudera.org:8080/7921 Reviewed-by: Dan Burkert --- M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java 1 file changed, 6 insertions(+), 5 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/7921 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Id1c54bcecc3e13ae64dd90efe6cf53021517dcdf Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-1.5.x Gerrit-Owner: Mike Percy Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR] Implement a lock table
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/7918 to look at the new patch set (#2). Change subject: Implement a lock table .. Implement a lock table This lock table offers the abstraction of an unlimited number of locks that can be used like reservations without holding a centralized mutex. An example of a use case where this is useful is an entry point where a task is begun that may take some time while duplicate or conflicting requests may arrive. In such a case, a slot may be reserved in the lock table corresponding to that task so that duplicate or conflicting requests can be rejected instead of serviced. This initial implementation only offers a TryLock() interface. A blocking Lock() method would be possible to efficiently implement in the future, if needed, providing the underlying lock implementation was changed from a spinlock to a mutex. Change-Id: I0404478675e079ba147fbdb4b0bac9db0cdc87e5 --- M src/kudu/util/CMakeLists.txt A src/kudu/util/lock_table-test.cc A src/kudu/util/lock_table.h 3 files changed, 205 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/7918/2 -- To view, visit http://gerrit.cloudera.org:8080/7918 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0404478675e079ba147fbdb4b0bac9db0cdc87e5 Gerrit-PatchSet: 2 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
