[kudu-CR](gh-pages) www: Add list of committers to the site
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3721 to review the following change. Change subject: www: Add list of committers to the site .. www: Add list of committers to the site Change-Id: Ie3a246e4fdb4656dda4485dcccd3e04bec04b39d --- M _includes/top_common.html A committers.md M community.md M css/kudu.css 4 files changed, 38 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/3721/1 -- To view, visit http://gerrit.cloudera.org:8080/3721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie3a246e4fdb4656dda4485dcccd3e04bec04b39d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) www: Add list of committers to the site
Mike Percy has posted comments on this change. Change subject: www: Add list of committers to the site .. Patch Set 1: Rendered HTML: http://mpercy.github.io/kudu/committers.html -- To view, visit http://gerrit.cloudera.org:8080/3721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a246e4fdb4656dda4485dcccd3e04bec04b39d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Remove ASF incubation callouts
Mike Percy has posted comments on this change. Change subject: Remove ASF incubation callouts .. Patch Set 1: Code-Review+2 If you want to update the mailing list archive links in a follow-up commit, that's fine with me too. -- To view, visit http://gerrit.cloudera.org:8080/3699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic71f46177e20a7aa09deccb098841c70eaa5f289 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) www: Add list of committers to the site
Mike Percy has submitted this change and it was merged. Change subject: www: Add list of committers to the site .. www: Add list of committers to the site Change-Id: Ie3a246e4fdb4656dda4485dcccd3e04bec04b39d Reviewed-on: http://gerrit.cloudera.org:8080/3721 Reviewed-by: Todd Lipcon Tested-by: Mike Percy --- M _includes/top_common.html A committers.md M community.md M css/kudu.css 4 files changed, 38 insertions(+), 1 deletion(-) Approvals: Mike Percy: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/3721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie3a246e4fdb4656dda4485dcccd3e04bec04b39d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) www: Add list of committers to the site
Mike Percy has posted comments on this change. Change subject: www: Add list of committers to the site .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3721 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a246e4fdb4656dda4485dcccd3e04bec04b39d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](gh-pages) Update the overview page
Mike Percy has posted comments on this change. Change subject: Update the overview page .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3726 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f071539bc1245fc63b6e6be210497d709f2f027 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Fix mailing list archive links to not reference incubator
Mike Percy has posted comments on this change. Change subject: Fix mailing list archive links to not reference incubator .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I994f774b0019c5f2018df7d8c6dc8992f134fa86 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) Update the overview page
Mike Percy has posted comments on this change. Change subject: Update the overview page .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3726 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0f071539bc1245fc63b6e6be210497d709f2f027 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Fix mailing list archive links to not reference incubator
Mike Percy has submitted this change and it was merged. Change subject: Fix mailing list archive links to not reference incubator .. Fix mailing list archive links to not reference incubator Change-Id: I994f774b0019c5f2018df7d8c6dc8992f134fa86 Reviewed-on: http://gerrit.cloudera.org:8080/3725 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M RELEASING.adoc M docs/release_notes.adoc 2 files changed, 3 insertions(+), 3 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3725 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I994f774b0019c5f2018df7d8c6dc8992f134fa86 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) Update the overview page
Mike Percy has submitted this change and it was merged. Change subject: Update the overview page .. Update the overview page Long-overdue updates to the overview page, including our new ASF status, official Python support, etc. Change-Id: I0f071539bc1245fc63b6e6be210497d709f2f027 Reviewed-on: http://gerrit.cloudera.org:8080/3726 Reviewed-by: Mike Percy Tested-by: Todd Lipcon Tested-by: Mike Percy --- A img/asf_logo.png M overview.html 2 files changed, 12 insertions(+), 20 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/3726 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I0f071539bc1245fc63b6e6be210497d709f2f027 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] C++ client: deprecating KuduPartialRow::SetString()
Mike Percy has posted comments on this change.
Change subject: C++ client: deprecating KuduPartialRow::SetString()
..
Patch Set 3:
(5 comments)
Feel free to hold off on doing reordering stuff until we have consensus on the
ABI issue. Then again, if both Todd and Dan are away next week then whoever is
being responsive on Gerrit probably wins... :)
http://gerrit.cloudera.org:8080/#/c/3723/3//COMMIT_MSG
Commit Message:
Line 21: An alternative approach might be to change behavior of
> I also like that approach (that was the initial idea I had), but after some
That would break ABI compatibility and it doesn't seem to be really necessary
in this case. I think we should take ABI breaks very seriously and only do it
when we must. Also if we are to follow semver then we need to bump the major
version to do this.
That said, I don't know of anybody other than Impala actually using the C++ API
in the data path, so if we were going to do it then we need to do it before 1.0
My personal preference is the current approach though
http://gerrit.cloudera.org:8080/#/c/3723/3/src/kudu/common/partial_row.cc
File src/kudu/common/partial_row.cc:
Line 289: Status KuduPartialRow::SetBinaryNoCopy(const Slice& col_name, const
Slice& val) {
style note: If you change the order in the header file please also change the
order in the cc file to match
http://gerrit.cloudera.org:8080/#/c/3723/3/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
Line 98: // Set the string/binary value but does not copy the value. The slice
style nit: It makes more sense to me to keep all the string methods together
and then keep all the binary methods together.
Line 109: ATTRIBUTE_DEPRECATED(
style nit: Line continuations should be 4 spaces, not 2. See
https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions
Line 110: "use SetStringNoCopy() instead; also consider using
SetStringCopy()");
style: Another line continuation, add an additional 4 spaces more than the
previous line is indented per
https://google.github.io/styleguide/cppguide.html#Function_Calls
--
To view, visit http://gerrit.cloudera.org:8080/3723
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I348b0b9437b8d7928e3b607a0e0610d8d0c58f7c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] C++ client: deprecating KuduPartialRow::SetString()
Mike Percy has posted comments on this change. Change subject: C++ client: deprecating KuduPartialRow::SetString() .. Patch Set 3: One more thing. If we go forward w/ the deprecation approach then we should add a short unit test that calls the deprecated methods. The compiler warnings on such a test can be squelched on recent versions of GCC (4.2+ I think) and Clang using #pragma GCC diagnostic ignore: http://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas -- To view, visit http://gerrit.cloudera.org:8080/3723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I348b0b9437b8d7928e3b607a0e0610d8d0c58f7c Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [make site.h] added C++ client API documentaion
Mike Percy has posted comments on this change. Change subject: [make_site.h] added C++ client API documentaion .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3729/1/docs/support/scripts/make_site.sh File docs/support/scripts/make_site.sh: Line 80: rm -Rf "$SITE_OUTPUT_DIR/cpp_client_api" Please use hyphens instead of underscores to separate the words (for SEO reasons per http://www.ecreativeim.com/blog/2011/03/seo-basics-hyphen-or-underscore-for-seo-urls/ ) so how about "cpp-client-api" here and below -- To view, visit http://gerrit.cloudera.org:8080/3729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9347f515ed4e1e0e3a5d46c969e57fe4983f17e2 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [make site.h] added C++ client API documentaion
Mike Percy has submitted this change and it was merged. Change subject: [make_site.h] added C++ client API documentaion .. [make_site.h] added C++ client API documentaion The C++ client API documentation is auto-generated by doxygen. The top-level file is 'index.html' file in the 'cpp_client_api' sub-directory of the site contents. Change-Id: I9347f515ed4e1e0e3a5d46c969e57fe4983f17e2 Reviewed-on: http://gerrit.cloudera.org:8080/3729 Reviewed-by: Mike Percy Tested-by: Mike Percy --- M docs/support/scripts/make_site.sh 1 file changed, 6 insertions(+), 2 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/3729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I9347f515ed4e1e0e3a5d46c969e57fe4983f17e2 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [make site.h] added C++ client API documentaion
Mike Percy has posted comments on this change. Change subject: [make_site.h] added C++ client API documentaion .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9347f515ed4e1e0e3a5d46c969e57fe4983f17e2 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [make site.h] added C++ client API documentaion
Mike Percy has posted comments on this change. Change subject: [make_site.h] added C++ client API documentaion .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9347f515ed4e1e0e3a5d46c969e57fe4983f17e2 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](gh-pages) Blog post for the graduation
Mike Percy has posted comments on this change. Change subject: Blog post for the graduation .. Patch Set 1: Mind pushing a rendered version for review? -- To view, visit http://gerrit.cloudera.org:8080/3741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic63d971c60377a4548b0ef71340e1d02db5a1ccf Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) Blog post for the graduation
Mike Percy has posted comments on this change. Change subject: Blog post for the graduation .. Patch Set 1: Thanks for pushing. Looks like we're missing line breaks on the section headers like "Availability and Oversight", "About the Apache Incubator", etc. -- To view, visit http://gerrit.cloudera.org:8080/3741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic63d971c60377a4548b0ef71340e1d02db5a1ccf Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) Blog post for the graduation
Mike Percy has submitted this change and it was merged. Change subject: Blog post for the graduation .. Blog post for the graduation This is just a reblog with the appropriate markdown syntax. Change-Id: Ic63d971c60377a4548b0ef71340e1d02db5a1ccf Reviewed-on: http://gerrit.cloudera.org:8080/3741 Reviewed-by: Mike Percy Tested-by: Mike Percy --- A _posts/2016-07-25-asf-graduation.md 1 file changed, 42 insertions(+), 0 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/3741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ic63d971c60377a4548b0ef71340e1d02db5a1ccf Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) Blog post for the graduation
Mike Percy has posted comments on this change. Change subject: Blog post for the graduation .. Patch Set 2: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic63d971c60377a4548b0ef71340e1d02db5a1ccf Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) www: Redirect kudu.i.a.o to kudu.a.o
Hello Jean-Daniel Cryans, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3756 to review the following change. Change subject: www: Redirect kudu.i.a.o to kudu.a.o .. www: Redirect kudu.i.a.o to kudu.a.o Change-Id: I21dde713c70c40a0ebf6f1b4478cf4dfd81de86d --- M .htaccess 1 file changed, 10 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/3756/1 -- To view, visit http://gerrit.cloudera.org:8080/3756 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I21dde713c70c40a0ebf6f1b4478cf4dfd81de86d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR](gh-pages) www: Redirect kudu.i.a.o to kudu.a.o
Mike Percy has posted comments on this change. Change subject: www: Redirect kudu.i.a.o to kudu.a.o .. Patch Set 1: Code-Review+2 Verified+1 Thanks for your vote of confidence, JD ;) -- To view, visit http://gerrit.cloudera.org:8080/3756 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I21dde713c70c40a0ebf6f1b4478cf4dfd81de86d Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) www: Redirect kudu.i.a.o to kudu.a.o
Mike Percy has submitted this change and it was merged. Change subject: www: Redirect kudu.i.a.o to kudu.a.o .. www: Redirect kudu.i.a.o to kudu.a.o Change-Id: I21dde713c70c40a0ebf6f1b4478cf4dfd81de86d Reviewed-on: http://gerrit.cloudera.org:8080/3756 Reviewed-by: Mike Percy Tested-by: Mike Percy --- M .htaccess 1 file changed, 10 insertions(+), 0 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/3756 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I21dde713c70c40a0ebf6f1b4478cf4dfd81de86d Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) Add weekly update for 07/25
Mike Percy has posted comments on this change. Change subject: Add weekly update for 07/25 .. Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/3763/1/_posts/2016-07-26-weekly-update.md File _posts/2016-07-26-weekly-update.md: Line 21: in place of `org.kududb`. This is done in a non-backward compatible way, meaning that existing How about tweak this a bit: This was done in a **backward-incompatible** way, meaning that import statements will have to be modified in existing Java code to compile against a newer Kudu JAR version (from 0.10.0 onward). Note that wire compatibility is not affected, and applications compiled against Kudu 0.9.0 will still be able to communicate with servers running Kudu 0.10.0, and vice versa. PS1, Line 26: completely changes how Exceptions are managed This sounds scary. Can you add a sentence or two that talks about what existing applications will have to do to deal with this change when they upgrade to 0.10.0+ ? PS1, Line 32: new API documentation new API documentation for C++ developers PS1, Line 38: It’s s/It's/`ksck` is/ PS1, Line 42: many rounds of reviews many rounds of reviews -> many rounds of review PS1, Line 44: the nit: this functionality -- To view, visit http://gerrit.cloudera.org:8080/3763 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95de597ca5e33c05ec4f820315952310f07983a0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Add weekly update for 07/25
Mike Percy has posted comments on this change. Change subject: Add weekly update for 07/25 .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3763/1/_posts/2016-07-26-weekly-update.md File _posts/2016-07-26-weekly-update.md: Line 21: in place of `org.kududb`. This is done in a non-backward compatible way, meaning that existing > AFAIK we don't know for sure if wire compatibility is broken or not, I sure OK, how about add: Note that wire compatibility is not affected by this change. -- To view, visit http://gerrit.cloudera.org:8080/3763 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95de597ca5e33c05ec4f820315952310f07983a0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR](gh-pages) Add weekly update for 07/25
Mike Percy has posted comments on this change. Change subject: Add weekly update for 07/25 .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/3763/2/_posts/2016-07-26-weekly-update.md File _posts/2016-07-26-weekly-update.md: PS2, Line 32: hat use that uses PS2, Line 33: with more specific with a more specific -- To view, visit http://gerrit.cloudera.org:8080/3763 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95de597ca5e33c05ec4f820315952310f07983a0 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] [make site.sh] an option for the script: doxygen
Mike Percy has posted comments on this change. Change subject: [make_site.sh] an option for the script: doxygen .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3766 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I23394d4339b9afb696335d9795f11d802ff9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] [make site.sh] an option for the script: doxygen
Mike Percy has submitted this change and it was merged. Change subject: [make_site.sh] an option for the script: doxygen .. [make_site.sh] an option for the script: doxygen By default, do not build the doxygen docs for the site. Building auto-generated doxygen docs requires the 'doxygen' tool to be present at build machines, which is not the case right now. To build doxygen docs, add 'doxygen' option when running the script. The latter happens automatically for the 'site' target when the doxygen package is present at the build machine. Change-Id: I23394d4339b9afb696335d9795f11d802ff9 Reviewed-on: http://gerrit.cloudera.org:8080/3766 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M CMakeLists.txt M docs/support/scripts/make_site.sh 2 files changed, 42 insertions(+), 13 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3766 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I23394d4339b9afb696335d9795f11d802ff9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy
[kudu-CR](gh-pages) Add weekly update for 07/25
Mike Percy has posted comments on this change. Change subject: Add weekly update for 07/25 .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3763 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I95de597ca5e33c05ec4f820315952310f07983a0 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Fix zip command in make site.sh
Hello Adar Dembo, Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3780 to review the following change. Change subject: Fix zip command in make_site.sh .. Fix zip command in make_site.sh Also make a few more minor changes: * Run doxygen by default so people updating the site don't tend to forget to run it, but retain an option to skip it * Add a --help option to the command Tested on Ubuntu 16.04 in both default and --no-doxygen modes, with and without doxygen installed. Change-Id: I3332fc36c9e33baed82f3eabfb5866a9cd41422f --- M docs/support/scripts/make_site.sh 1 file changed, 22 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/3780/1 -- To view, visit http://gerrit.cloudera.org:8080/3780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3332fc36c9e33baed82f3eabfb5866a9cd41422f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin
[kudu-CR] Fix a couple bugs in make site.sh
Mike Percy has uploaded a new patch set (#2). Change subject: Fix a couple bugs in make_site.sh .. Fix a couple bugs in make_site.sh This patch fixes a couple minor script bugs: * zip command should not quote arguments * doxygen output appears in build dir, not source dir Also makes a couple other minor changes: * Run doxygen by default so people updating the site don't tend to forget to run it, but retain an option to skip it * Add a --help option to the command Tested on Ubuntu 16.04 in both default and --no-doxygen modes, with and without doxygen installed. Change-Id: I3332fc36c9e33baed82f3eabfb5866a9cd41422f --- M docs/support/scripts/make_site.sh 1 file changed, 22 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/3780/2 -- To view, visit http://gerrit.cloudera.org:8080/3780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3332fc36c9e33baed82f3eabfb5866a9cd41422f 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
[kudu-CR] Fix a couple bugs in make site.sh
Mike Percy has posted comments on this change. Change subject: Fix a couple bugs in make_site.sh .. Patch Set 2: > So you'll need to modify Kudu-Docs to pass --no-doxygen until we > get doxygen installed in those build images, right? That's not really relevant to upstream, but yes, exactly. -- To view, visit http://gerrit.cloudera.org:8080/3780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3332fc36c9e33baed82f3eabfb5866a9cd41422f 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] Fix a couple bugs in make site.sh
Mike Percy has submitted this change and it was merged. Change subject: Fix a couple bugs in make_site.sh .. Fix a couple bugs in make_site.sh This patch fixes a couple minor script bugs: * zip command should not quote arguments * doxygen output appears in build dir, not source dir Also makes a couple other minor changes: * Run doxygen by default so people updating the site don't tend to forget to run it, but retain an option to skip it * Add a --help option to the command Tested on Ubuntu 16.04 in both default and --no-doxygen modes, with and without doxygen installed. Change-Id: I3332fc36c9e33baed82f3eabfb5866a9cd41422f Reviewed-on: http://gerrit.cloudera.org:8080/3780 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M docs/support/scripts/make_site.sh 1 file changed, 22 insertions(+), 9 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3780 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3332fc36c9e33baed82f3eabfb5866a9cd41422f 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] Import CallbackList from chromium-base
Mike Percy has abandoned this change. Change subject: Import CallbackList from chromium-base .. Abandoned we didn't end up needing this stuff. also, now that Kudu supports C++11, we could import the variadic template version if we wanted to -- To view, visit http://gerrit.cloudera.org:8080/997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I489fd5598a1108ce5b2a379f0337f411b82e0810 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy
[kudu-CR] Changes to import CallbackList from chromium-base
Mike Percy has abandoned this change. Change subject: Changes to import CallbackList from chromium-base .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/998 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I11b2d37da026661fe284c15b6e92ade4f2edb250 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy
[kudu-CR] Add support to Env for creation of a temp dir
Mike Percy has abandoned this change. Change subject: Add support to Env for creation of a temp dir .. Abandoned didn't end up needing this -- To view, visit http://gerrit.cloudera.org:8080/996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: Icc8ff8653f9b63082ed541205b526a2e3c85ee18 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy
[kudu-CR] KUDU-526: use on-disk cmeta when loading existing master state
Mike Percy has posted comments on this change.
Change subject: KUDU-526: use on-disk cmeta when loading existing master state
..
Patch Set 1: Code-Review+2
(1 comment)
This patch looks fine to me. It obviously converts the gflags from a command to
a suggestion on startup, but we already knew that was coming. Needs docs, but
it's reasonable to defer than until we're done changing stuff to do.
http://gerrit.cloudera.org:8080/#/c/3786/1/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:
Line 330: TEST_F(MasterFailoverTest, TestKUDU526) {
nit: how about TestMasterUUIDResolution or something (you can add a comment
about KUDU-526 if you want but I'm not sure it adds any incremental clarity)
--
To view, visit http://gerrit.cloudera.org:8080/3786
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5b4c6d8b6adf696973445a6f9d1314ba9de27e70
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] KUDU-526: use on-disk cmeta when loading existing master state
Mike Percy has posted comments on this change. Change subject: KUDU-526: use on-disk cmeta when loading existing master state .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5b4c6d8b6adf696973445a6f9d1314ba9de27e70 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Adding some missing 0.10.0 release notes
Mike Percy has posted comments on this change. Change subject: Adding some missing 0.10.0 release notes .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3802 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibdc9fd57b05434874845ffa9c0ff905b5b8d0422 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: No
[kudu-CR] KUDU-1517 Implement doc feedback from Sue M
Mike Percy has posted comments on this change. Change subject: KUDU-1517 Implement doc feedback from Sue M .. Patch Set 1: (11 comments) http://gerrit.cloudera.org:8080/#/c/3638/1/docs/index.adoc File docs/index.adoc: PS1, Line 73: mechanism s/mechanisms/syntax/ PS1, Line 74: any other table with HDFS or HBase persistenc as any other Impala table, like those using HDFS or HBase for persistence. PS1, Line 78: solutions s/solutions/standards/ PS1, Line 78: to s/to/with/ PS1, Line 79: FROM I would say: you can specify complex joins with a FROM clause in a subquery However, I am not sure if it's true that this works for updates with Impala and Kudu. Are you sure? I suppose this means you can do something like what is shown in this SO article: http://stackoverflow.com/questions/1293330/how-can-i-do-an-update-statement-with-join-in-sql PS1, Line 89: within s/within/used by/ PS1, Line 89: to s/to/across/ Line 108: Data Compression:: Because a given column contains only one type of data, pattern-based compression can nit: Would be nice to keep this line under 100 chars long PS1, Line 185: when using s/when using/in/ PS1, Line 236: and nit: and and http://gerrit.cloudera.org:8080/#/c/3638/1/docs/release_notes.adoc File docs/release_notes.adoc: Line 31: == Introduction nit: I think this should be a 3rd-level heading -- To view, visit http://gerrit.cloudera.org:8080/3638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a7647b3e5d4d36e82e06ce02a45a8811e4efed3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Misty Stanley-Jones Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Fix flakiness in RaftConsensusITest.TestReplaceChangeConfigOperation
Hello Dinesh Bhat,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/3819
to review the following change.
Change subject: Fix flakiness in
RaftConsensusITest.TestReplaceChangeConfigOperation
..
Fix flakiness in RaftConsensusITest.TestReplaceChangeConfigOperation
This test was occasionally failing its 2nd election in TSAN mode because
we were resuming the previous leader before the new leader could be
elected. Sometimes the previous leader was fast enough to replicate its
pending config change to a majority of nodes before the new candidate
could send out its election RPC, thus violating the underlying
assumptions of this test.
I also added a minor C++11 syntax-only change in the cluster itest utils
class as a part of this patch (doesn't change any behavior).
Before this fix, this test failed 15/800 times on the dist-test cluster.
After this change, it passed 100% of the time.
The test log looked something like this:
I0729 18:59:47.834403 11544 raft_consensus.cc:370] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9 [term 1
FOLLOWER]: No leader contacted us within the election timeout. Triggering
leader election
I0729 18:59:47.834686 11544 raft_consensus.cc:2019] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9 [term 1
FOLLOWER]: Advancing to term 2
I0729 18:59:47.840427 11544 leader_election.cc:223] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Requesting vote from peer
54197053abab4b6cb1b1632c9d1062dc
I0729 18:59:47.840860 11544 leader_election.cc:223] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Requesting vote from peer
3522a8de8170476dba0beb58cb2150d4
I0729 18:59:47.872720 11669 raft_consensus.cc:869] T
e3503c47a21649ca931234999cd0bb45 P 3522a8de8170476dba0beb58cb2150d4 [term 1
FOLLOWER]: Refusing update from remote peer 54197053abab4b6cb1b1632c9d1062dc:
Log matching property violated. Preceding OpId in replica: term: 1 index: 1.
Preceding OpId from leader: term: 1 index: 2. (index mismatch)
I0729 18:59:47.874522 11454 consensus_queue.cc:578] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [LEADER]:
Connected to new peer: Peer: 3522a8de8170476dba0beb58cb2150d4, Is new: false,
Last received: 1.1, Next index: 2, Last known committed idx: 1, Last exchange
result: ERROR, Needs remote bootstrap: false
I0729 18:59:47.878105 11150 raft_consensus.cc:1324] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Handling vote request from an unknown peer
d4f64819170a4cf78fe4c9e9a72ec4b9
I0729 18:59:47.878290 11150 raft_consensus.cc:2014] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Stepping down as leader of term 1
I0729 18:59:47.878451 11150 raft_consensus.cc:499] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Becoming Follower/Learner. State: Replica:
54197053abab4b6cb1b1632c9d1062dc, State: 1, Role: LEADER
Watermarks: {Received: term: 1 index: 2 Committed: term: 1 index: 1}
I0729 18:59:47.878968 11150 consensus_queue.cc:162] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc
[NON_LEADER]: Queue going to NON_LEADER mode. State: All replicated op: 0.0,
Majority replicated op: 1.1, Committed index: 1.1, Last appended: 1.2, Current
term: 1, Majority size: -1, State: 1, Mode: NON_LEADER
I0729 18:59:47.879871 11150 consensus_peers.cc:358] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc -> Peer
3522a8de8170476dba0beb58cb2150d4 (127.37.56.2:53243): Closing peer:
3522a8de8170476dba0beb58cb2150d4
I0729 18:59:47.882057 11150 raft_consensus.cc:2019] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
FOLLOWER]: Advancing to term 2
I0729 18:59:47.885711 11150 raft_consensus.cc:1626] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 2
FOLLOWER]: Leader election vote request: Denying vote to candidate
d4f64819170a4cf78fe4c9e9a72ec4b9 for term 2 because replica has last-logged
OpId of term: 1 index: 2, which is greater than that of the candidate, which
has last-logged OpId of term: 1 index: 1.
I0729 18:59:47.892060 11477 leader_election.cc:361] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Vote denied by peer
54197053abab4b6cb1b1632c9d1062dc. Message: Invalid argument: T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 2
FOLLOWER]: Leader election vote request: Denying vote to candidate
d4f64819170a4cf78fe4c9e9a72ec4b9 for term 2 because replica has last-logged
OpId of term: 1 index: 2, which is greater than that of the candidate, which
has last-logged OpId of term: 1 index: 1.
I0729 18:59:47.894548 11669 raft_consens
[kudu-CR] Fix flakiness in RaftConsensusITest.TestReplaceChangeConfigOperation
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/3819
to look at the new patch set (#2).
Change subject: Fix flakiness in
RaftConsensusITest.TestReplaceChangeConfigOperation
..
Fix flakiness in RaftConsensusITest.TestReplaceChangeConfigOperation
This test was occasionally failing its 2nd election in TSAN mode because
we were resuming the previous leader before the new leader could be
elected. Sometimes the previous leader was fast enough to replicate its
pending config change to a majority of nodes before the new candidate
could send out its election RPC, thus violating the underlying
assumptions of this test.
I also added a minor C++11 syntax-only change in the cluster itest utils
class as a part of this patch (doesn't change any behavior).
Before this fix, this test failed 15/800 times on the dist-test cluster.
After this change, it passed 100% of the time.
The test log looked something like this:
I0729 18:59:47.834403 11544 raft_consensus.cc:370] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9 [term 1
FOLLOWER]: No leader contacted us within the election timeout. Triggering
leader election
I0729 18:59:47.834686 11544 raft_consensus.cc:2019] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9 [term 1
FOLLOWER]: Advancing to term 2
I0729 18:59:47.840427 11544 leader_election.cc:223] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Requesting vote from peer
54197053abab4b6cb1b1632c9d1062dc
I0729 18:59:47.840860 11544 leader_election.cc:223] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Requesting vote from peer
3522a8de8170476dba0beb58cb2150d4
I0729 18:59:47.872720 11669 raft_consensus.cc:869] T
e3503c47a21649ca931234999cd0bb45 P 3522a8de8170476dba0beb58cb2150d4 [term 1
FOLLOWER]: Refusing update from remote peer 54197053abab4b6cb1b1632c9d1062dc:
Log matching property violated. Preceding OpId in replica: term: 1 index: 1.
Preceding OpId from leader: term: 1 index: 2. (index mismatch)
I0729 18:59:47.874522 11454 consensus_queue.cc:578] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [LEADER]:
Connected to new peer: Peer: 3522a8de8170476dba0beb58cb2150d4, Is new: false,
Last received: 1.1, Next index: 2, Last known committed idx: 1, Last exchange
result: ERROR, Needs remote bootstrap: false
I0729 18:59:47.878105 11150 raft_consensus.cc:1324] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Handling vote request from an unknown peer
d4f64819170a4cf78fe4c9e9a72ec4b9
I0729 18:59:47.878290 11150 raft_consensus.cc:2014] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Stepping down as leader of term 1
I0729 18:59:47.878451 11150 raft_consensus.cc:499] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Becoming Follower/Learner. State: Replica:
54197053abab4b6cb1b1632c9d1062dc, State: 1, Role: LEADER
Watermarks: {Received: term: 1 index: 2 Committed: term: 1 index: 1}
I0729 18:59:47.878968 11150 consensus_queue.cc:162] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc
[NON_LEADER]: Queue going to NON_LEADER mode. State: All replicated op: 0.0,
Majority replicated op: 1.1, Committed index: 1.1, Last appended: 1.2, Current
term: 1, Majority size: -1, State: 1, Mode: NON_LEADER
I0729 18:59:47.879871 11150 consensus_peers.cc:358] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc -> Peer
3522a8de8170476dba0beb58cb2150d4 (127.37.56.2:53243): Closing peer:
3522a8de8170476dba0beb58cb2150d4
I0729 18:59:47.882057 11150 raft_consensus.cc:2019] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
FOLLOWER]: Advancing to term 2
I0729 18:59:47.885711 11150 raft_consensus.cc:1626] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 2
FOLLOWER]: Leader election vote request: Denying vote to candidate
d4f64819170a4cf78fe4c9e9a72ec4b9 for term 2 because replica has last-logged
OpId of term: 1 index: 2, which is greater than that of the candidate, which
has last-logged OpId of term: 1 index: 1.
I0729 18:59:47.892060 11477 leader_election.cc:361] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Vote denied by peer
54197053abab4b6cb1b1632c9d1062dc. Message: Invalid argument: T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 2
FOLLOWER]: Leader election vote request: Denying vote to candidate
d4f64819170a4cf78fe4c9e9a72ec4b9 for term 2 because replica has last-logged
OpId of term: 1 index: 2, which is greater than that of the candidate, which
has last-logged OpId of term: 1 index: 1.
I0729 18:59:47.894548 11669 raft_c
[kudu-CR] Fix flakiness in RaftConsensusITest.TestReplaceChangeConfigOperation
Mike Percy has posted comments on this change.
Change subject: Fix flakiness in
RaftConsensusITest.TestReplaceChangeConfigOperation
..
Patch Set 2:
(1 comment)
> RaftConsensusITest(line 594)
That's fine without waiting because the client will automatically retry until
the election is done.
> RemoteBootStrapITest and DeleteTabletTest
Yeah, same as above. This case was problematic because of the nature of the
test, which is fairly unique.
http://gerrit.cloudera.org:8080/#/c/3819/2/src/kudu/integration-tests/cluster_itest_util.cc
File src/kudu/integration-tests/cluster_itest_util.cc:
Line 141: RETURN_NOT_OK(GetLastOpIdForEachReplica(tablet_id, { replica },
opid_type, timeout, &op_ids));
> Was this styling issue or any impact in the way vector is passed in ?
Just a style issue (see commit message) ... less lines == easier to read.
--
To view, visit http://gerrit.cloudera.org:8080/3819
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib91b5cc974656e82f670d6a938f537b63338d036
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] Fix flakiness in RaftConsensusITest.TestReplaceChangeConfigOperation
Mike Percy has posted comments on this change. Change subject: Fix flakiness in RaftConsensusITest.TestReplaceChangeConfigOperation .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3819/2//COMMIT_MSG Commit Message: Line 47: > Also a nit: You may want to link this commit to KUDU-1548 I triaged where I Notice that the rest of the commit message is wrapped at 80 chars. I'm not going to try to condense this part to 80 chars -- it will be totally unreadable. Generally speaking, it's not good style to go over 80 chars, but I think one reasonable exception is pasting in log messages like this one. It's at the bottom of the commit message, so by the time you get to this part in "git log" you've already read the important part. Regarding KUDU-1548 thanks for filing it. It looks like that JIRA potentially has a bunch of different issues listed. It might be better to have separate JIRAs about each issue though. I'll add a link to it in the commit message. -- To view, visit http://gerrit.cloudera.org:8080/3819 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib91b5cc974656e82f670d6a938f537b63338d036 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] KUDU-1548. Fix flaky RaftConsensusITest.TestReplaceChangeConfigOperation
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/3819
to look at the new patch set (#3).
Change subject: KUDU-1548. Fix flaky
RaftConsensusITest.TestReplaceChangeConfigOperation
..
KUDU-1548. Fix flaky RaftConsensusITest.TestReplaceChangeConfigOperation
This test was occasionally failing its 2nd election in TSAN mode because
we were resuming the previous leader before the new leader could be
elected. Sometimes the previous leader was fast enough to replicate its
pending config change to a majority of nodes before the new candidate
could send out its election RPC, thus violating the underlying
assumptions of this test.
I also added a minor C++11 syntax-only change in the cluster itest utils
class as a part of this patch (doesn't change any behavior).
Before this fix, this test failed 15/800 times on the dist-test cluster.
After this change, it passed 100% of the time.
The test log looked something like this:
I0729 18:59:47.834403 11544 raft_consensus.cc:370] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9 [term 1
FOLLOWER]: No leader contacted us within the election timeout. Triggering
leader election
I0729 18:59:47.834686 11544 raft_consensus.cc:2019] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9 [term 1
FOLLOWER]: Advancing to term 2
I0729 18:59:47.840427 11544 leader_election.cc:223] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Requesting vote from peer
54197053abab4b6cb1b1632c9d1062dc
I0729 18:59:47.840860 11544 leader_election.cc:223] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Requesting vote from peer
3522a8de8170476dba0beb58cb2150d4
I0729 18:59:47.872720 11669 raft_consensus.cc:869] T
e3503c47a21649ca931234999cd0bb45 P 3522a8de8170476dba0beb58cb2150d4 [term 1
FOLLOWER]: Refusing update from remote peer 54197053abab4b6cb1b1632c9d1062dc:
Log matching property violated. Preceding OpId in replica: term: 1 index: 1.
Preceding OpId from leader: term: 1 index: 2. (index mismatch)
I0729 18:59:47.874522 11454 consensus_queue.cc:578] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [LEADER]:
Connected to new peer: Peer: 3522a8de8170476dba0beb58cb2150d4, Is new: false,
Last received: 1.1, Next index: 2, Last known committed idx: 1, Last exchange
result: ERROR, Needs remote bootstrap: false
I0729 18:59:47.878105 11150 raft_consensus.cc:1324] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Handling vote request from an unknown peer
d4f64819170a4cf78fe4c9e9a72ec4b9
I0729 18:59:47.878290 11150 raft_consensus.cc:2014] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Stepping down as leader of term 1
I0729 18:59:47.878451 11150 raft_consensus.cc:499] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
LEADER]: Becoming Follower/Learner. State: Replica:
54197053abab4b6cb1b1632c9d1062dc, State: 1, Role: LEADER
Watermarks: {Received: term: 1 index: 2 Committed: term: 1 index: 1}
I0729 18:59:47.878968 11150 consensus_queue.cc:162] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc
[NON_LEADER]: Queue going to NON_LEADER mode. State: All replicated op: 0.0,
Majority replicated op: 1.1, Committed index: 1.1, Last appended: 1.2, Current
term: 1, Majority size: -1, State: 1, Mode: NON_LEADER
I0729 18:59:47.879871 11150 consensus_peers.cc:358] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc -> Peer
3522a8de8170476dba0beb58cb2150d4 (127.37.56.2:53243): Closing peer:
3522a8de8170476dba0beb58cb2150d4
I0729 18:59:47.882057 11150 raft_consensus.cc:2019] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 1
FOLLOWER]: Advancing to term 2
I0729 18:59:47.885711 11150 raft_consensus.cc:1626] T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 2
FOLLOWER]: Leader election vote request: Denying vote to candidate
d4f64819170a4cf78fe4c9e9a72ec4b9 for term 2 because replica has last-logged
OpId of term: 1 index: 2, which is greater than that of the candidate, which
has last-logged OpId of term: 1 index: 1.
I0729 18:59:47.892060 11477 leader_election.cc:361] T
e3503c47a21649ca931234999cd0bb45 P d4f64819170a4cf78fe4c9e9a72ec4b9
[CANDIDATE]: Term 2 election: Vote denied by peer
54197053abab4b6cb1b1632c9d1062dc. Message: Invalid argument: T
e3503c47a21649ca931234999cd0bb45 P 54197053abab4b6cb1b1632c9d1062dc [term 2
FOLLOWER]: Leader election vote request: Denying vote to candidate
d4f64819170a4cf78fe4c9e9a72ec4b9 for term 2 because replica has last-logged
OpId of term: 1 index: 2, which is greater than that of the candidate, which
has last-logged OpId of term: 1 index: 1.
I0729 18:59:47.894548 1166
[kudu-CR](gh-pages) site tool: update repository URLs
Mike Percy has submitted this change and it was merged. Change subject: site_tool: update repository URLs .. site_tool: update repository URLs Change-Id: I5535e922ecf4f71b712b6d0dc1ce2d40d109870f Reviewed-on: http://gerrit.cloudera.org:8080/3827 Reviewed-by: Mike Percy Tested-by: Mike Percy --- M site_tool 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/3827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5535e922ecf4f71b712b6d0dc1ce2d40d109870f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) site tool: update repository URLs
Mike Percy has posted comments on this change. Change subject: site_tool: update repository URLs .. Patch Set 1: Code-Review+2 Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5535e922ecf4f71b712b6d0dc1ce2d40d109870f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1416 Upsert support for Flume sink
Mike Percy has posted comments on this change. Change subject: KUDU-1416 Upsert support for Flume sink .. Patch Set 2: Code-Review+2 (1 comment) Looks good to me. I'm going to just rebase this myself and push it unless I run into any unexpected issues. http://gerrit.cloudera.org:8080/#/c/3157/1//COMMIT_MSG Commit Message: Line 15: Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85 There is a lot that we could do to make this even better, but I think having this example is a good start. One thing we could do in a future patch is to make a very generic event producer that would read avro encoded events and automatically map those to a configured table, i.e. a "zero coding" path. -- To view, visit http://gerrit.cloudera.org:8080/3157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Ara Ebrahimi Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: Yes
[kudu-CR] KUDU-1416 Upsert support for Flume sink
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3157 to look at the new patch set (#3). Change subject: KUDU-1416 Upsert support for Flume sink .. KUDU-1416 Upsert support for Flume sink This patch adds a new SimpleKeyedKuduEventProducer class that can upsert. The original KuduEventProducer class, SimpleKuduEventProducer, assumed it was inserting a binary payload to a key column, and thus was not compatible with upserts. SimpleKeyedKuduEventProducer supports inserting or upserting a binary payload with string key column. Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85 --- M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduEventProducer.java M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduEventProducer.java A java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduEventProducerTest.java 4 files changed, 315 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/3157/3 -- To view, visit http://gerrit.cloudera.org:8080/3157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Ara Ebrahimi Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-1416 Upsert support for Flume sink
Mike Percy has posted comments on this change. Change subject: KUDU-1416 Upsert support for Flume sink .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Ara Ebrahimi Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-HasComments: No
[kudu-CR] KUDU-1416 Upsert support for Flume sink
Mike Percy has submitted this change and it was merged. Change subject: KUDU-1416 Upsert support for Flume sink .. KUDU-1416 Upsert support for Flume sink This patch adds a new SimpleKeyedKuduEventProducer class that can upsert. The original KuduEventProducer class, SimpleKuduEventProducer, assumed it was inserting a binary payload to a key column, and thus was not compatible with upserts. SimpleKeyedKuduEventProducer supports inserting or upserting a binary payload with string key column. Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85 Reviewed-on: http://gerrit.cloudera.org:8080/3157 Tested-by: Kudu Jenkins Reviewed-by: Mike Percy --- M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/KuduSink.java A java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKeyedKuduEventProducer.java M java/kudu-flume-sink/src/main/java/org/apache/kudu/flume/sink/SimpleKuduEventProducer.java A java/kudu-flume-sink/src/test/java/org/apache/kudu/flume/sink/KeyedKuduEventProducerTest.java 4 files changed, 315 insertions(+), 3 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibe5b5df70687103ed6916d58148336882aa66d85 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Ara Ebrahimi Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] make election timeout jitter more aggressive
Mike Percy has posted comments on this change. Change subject: make election timeout jitter more aggressive .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3828/1//COMMIT_MSG Commit Message: Line 7: make election timeout jitter more aggressive > yea, but the jitter is only aggressive due to the backoff being more aggres I agree with Todd, backoff cap avoids insane exponential backoffs. It seems like the jitter is what we are really worried about here. And TBH I'm not convinced this is the problem. Although I'd support wider jitter variance. On a sort of side note, I tried to add a generic exponential backoff helper a long time ago in https://gerrit.cloudera.org/#/c/979/ ... maybe we should partially resurrect that patch and work on ensuring that we have a single exponential backoff function that is parameterized and flexible enough to handle all situations. -- To view, visit http://gerrit.cloudera.org:8080/3828 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c9dad820c2b7d4bc4b9e791b78222559cdf63c8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Rename Remote Bootstrap to Tablet Copy (part 1)
Mike Percy has posted comments on this change. Change subject: Rename Remote Bootstrap to Tablet Copy (part 1) .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3848 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic017e9121ed365cb3d9c9c81430915bf5b295bf6 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Rename Remote Bootstrap: some manual changes
Mike Percy has posted comments on this change. Change subject: Rename Remote Bootstrap: some manual changes .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/3852/1/docs/design-docs/raft-remote-bootstrap.md File docs/design-docs/raft-remote-bootstrap.md: Line 58: ## Design & implementation of tablet Tablet Copy nit: tablet Tablet Copy. If this is too painful to rebase from then we can fix after -- To view, visit http://gerrit.cloudera.org:8080/3852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e2860b64e46c5cc021cb8574e6845d7bcb27cce Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR] Rename remote bootstrap files to 'tablet copy'
Mike Percy has posted comments on this change. Change subject: Rename remote bootstrap files to 'tablet copy' .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f31fb72d11d16ee05bf90be3a7c77a82e5db6f7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Rename Remote Bootstrap: some manual changes
Mike Percy has posted comments on this change. Change subject: Rename Remote Bootstrap: some manual changes .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e2860b64e46c5cc021cb8574e6845d7bcb27cce Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Mike Percy has posted comments on this change.
Change subject: KUDU-1500: Data race in
RaftConsensusITest.TestCorruptReplicaMetadata
..
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3823/4/src/kudu/tablet/tablet_metadata.h
File src/kudu/tablet/tablet_metadata.h:
Line 104: const Partition& partition() const {
Typically when we create functions like this we name them like:
GetPartitionUnlocked()
GetPartitionCopy()
And add thread-safety comments to them.
I'd even suggest going one step further. Have Tablet keep a copy of the
Partition field locally when it's constructed and never access it from the
TabletMetadata after that. That way all the calls that looks like
tablet->metadata()->partition_locked() etc will just look like
tablet->partition() and they don't have to worry about taking locks or copying.
Line 142: const PartitionSchema partition_schema_locked() const {
I don't think this is needed anymore if we make
PartitionSchema::PartitionDebugString() static
http://gerrit.cloudera.org:8080/#/c/3823/4/src/kudu/tserver/tserver-path-handlers.cc
File src/kudu/tserver/tserver-path-handlers.cc:
Line 209: string partition =
peer->tablet_metadata()->partition_schema_locked()
Can we just make PartitionSchema::PartitionDebugString() a static method?
--
To view, visit http://gerrit.cloudera.org:8080/3823
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dinesh Bhat
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3868/1//COMMIT_MSG
Commit Message:
Line 17: This is safe, but an incompatible change, but we don't bump
Code will still link / compile so this isn't API or ABI compatible. It's just a
semantics change.
http://gerrit.cloudera.org:8080/#/c/3868/1/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
Line 108: // must remain valid until data for corresponding RPC calls are
generated.
Isn't the rule that the data must remain valid until the corresponding RPC
calls are completed and as long as the error buffers may be accessed? Since I
think (but am not sure) that the error buffers would refer to the uncopied data.
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata
Mike Percy has posted comments on this change. Change subject: KUDU-1500: Data race in RaftConsensusITest.TestCorruptReplicaMetadata .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/3823/6/src/kudu/tserver/tserver-path-handlers.cc File src/kudu/tserver/tserver-path-handlers.cc: Line 211: peer->tablet_metadata()->schema()); Note: schema_ is also modified in TabletMetadata::LoadFromSuperBlock() -- To view, visit http://gerrit.cloudera.org:8080/3823 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If57617e22b41296b8d4e8ad131220f1ebb235019 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 2:
(3 comments)
Can we add basic test coverage for SetString() and SetBinary()?
http://gerrit.cloudera.org:8080/#/c/3868/2/src/kudu/client/stubs.h
File src/kudu/client/stubs.h:
Line 66: // For deprecated functions or variables, generate a warning at usage
sites.
If you want to bring in this helper as part of this patch, just mention it in
the commit message.
Also, I wonder why this is defined both in port.h and here... maybe we should
not define it here if it's not used in the client.
http://gerrit.cloudera.org:8080/#/c/3868/2/src/kudu/common/partial_row.cc
File src/kudu/common/partial_row.cc:
Line 277: return SetSliceCopy >(col_name, val);
Can we just delegate these calls to SetBinaryCopy() so it's clear it's just an
alias? Or vice versa?
http://gerrit.cloudera.org:8080/#/c/3868/2/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
Line 92: // Sets the binary/binary value, copying 'val' data immediately.
Essentially,
typo: binary/binary
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] [client][gutil] introduced ATTRIBUTE DEPRECATED
Mike Percy has submitted this change and it was merged.
Change subject: [client][gutil] introduced ATTRIBUTE_DEPRECATED
..
[client][gutil] introduced ATTRIBUTE_DEPRECATED
The ATTRIBUTE_DEPRECATED macro is to mark deprecated methods.
An example of usage:
void foo() ATTRIBUTE_DEPRECATED("use bar() instead");
Change-Id: Ib0d3c48fe8b5b599126ff45728d0b8da30ffe082
Reviewed-on: http://gerrit.cloudera.org:8080/3874
Reviewed-by: Todd Lipcon
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy
---
M src/kudu/client/stubs.h
M src/kudu/gutil/port.h
2 files changed, 24 insertions(+), 0 deletions(-)
Approvals:
Mike Percy: Looks good to me, approved
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3874
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0d3c48fe8b5b599126ff45728d0b8da30ffe082
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] [client][gutil] introduced ATTRIBUTE DEPRECATED
Mike Percy has posted comments on this change. Change subject: [client][gutil] introduced ATTRIBUTE_DEPRECATED .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3874 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib0d3c48fe8b5b599126ff45728d0b8da30ffe082 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3868/3/src/kudu/common/partial_row-test.cc
File src/kudu/common/partial_row-test.cc:
Line 189: TEST_F(PartialRowTest, TestSetBinaryAndString) {
TestSetBinaryAndString and TestSetBinaryAndStringCopy seem like mostly
copy/paste to me, can we avoid that by extracting a helper method?
Also, it would be nice to do SetString(2, src_str) and then have src_str go out
of scope before continuing. That way we get ASAN to do the work for us.
Line 262: ASSERT_OK(row.SetStringNoCopy(2, src_str));
It's up to you but personally I think NoCopy already has a lot of coverage in
existing tests and is less important to add coverage for.
Line 276: EXPECT_EQ(reinterpret_cast(src_str.data()),
I think this just asserts that the implementation works a certain way (i.e.
slices pointing to the same buffer). Personally I don't find that these types
of tests add a lot of value because they are so brittle. However if you want to
include this check then I'm alright with it.
Usually I would rather use something a little higher level to assert that the
contract provided by the interface is respected. That's easy to do with the
Copy() version and hard to do with the NoCopy() version, I think.
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3868/3/src/kudu/common/partial_row-test.cc
File src/kudu/common/partial_row-test.cc:
Line 189: TEST_F(PartialRowTest, TestSetBinaryAndString) {
> Yes, it's a good idea to have a helper method for those. Will do.
Well, it's undefined behavior what happens to a freed string but depending on
ASAN seems fine to me. The ASAN build runs before each commit, and really all
the time.
Here is what happens with ASAN when you use freed memory:
1. take ptr to heap and store it
2. free that memory -> ASAN marks the memory region as poisoned
3. access the pointed-to memory -> ASAN intercepts the memory access and
crashes the test with a bunch of debugging information
Line 262: ASSERT_OK(row.SetStringNoCopy(2, src_str));
> SetXxxNoCopy() are the newly introduced methods, so I think it's nice to ha
ah, that's fair. I noticed that all the existing uses of SetString() were
converted to use SetStringNoCopy() in this patch so considered that general
test coverage, if not direct unit test coverage.
Line 276: EXPECT_EQ(reinterpret_cast(src_str.data()),
> I thought this part was derived from the interface constraints for the Slic
That's a reasonable argument, no worries, I'm not really against keeping this
test.
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-HasComments: Yes
[kudu-CR](gh-pages) www: Add Kudu talks to Community page
Mike Percy has uploaded a new change for review. http://gerrit.cloudera.org:8080/3877 Change subject: www: Add Kudu talks to Community page .. www: Add Kudu talks to Community page Change-Id: I26fde3dabd5f08ac656f286118704bec82f27877 --- M community.md 1 file changed, 9 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/77/3877/1 -- To view, visit http://gerrit.cloudera.org:8080/3877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I26fde3dabd5f08ac656f286118704bec82f27877 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy
[kudu-CR](gh-pages) www: Add Kudu talks to Community page
Mike Percy has posted comments on this change. Change subject: www: Add Kudu talks to Community page .. Patch Set 1: Code-Review+2 Verified+1 Verified this looks good, pushing -- To view, visit http://gerrit.cloudera.org:8080/3877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I26fde3dabd5f08ac656f286118704bec82f27877 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) www: Add Kudu talks to Community page
Mike Percy has submitted this change and it was merged. Change subject: www: Add Kudu talks to Community page .. www: Add Kudu talks to Community page Change-Id: I26fde3dabd5f08ac656f286118704bec82f27877 Reviewed-on: http://gerrit.cloudera.org:8080/3877 Reviewed-by: Mike Percy Tested-by: Mike Percy --- M community.md 1 file changed, 9 insertions(+), 3 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/3877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I26fde3dabd5f08ac656f286118704bec82f27877 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3868/5/src/kudu/common/partial_row-test.cc
File src/kudu/common/partial_row-test.cc:
Line 41: Status (KuduPartialRow::* pmfSetter)(int, const Slice&),
Would you mind using std::function for this? Easier to read. Also, minor style
nit: don't use camelCaps for variable names.
Line 43: int column_idx, bool should_copy) {
nit: please use an enum instead of a bool for should_copy, like enum { COPY,
NO_COPY } or { kShouldCopy, kShouldNotCopy }, something like that. This makes
it obvious what the parameter meaning is at the call site.
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 9:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3868/9/src/kudu/common/partial_row-test.cc
File src/kudu/common/partial_row-test.cc:
Line 47: typedef Status (KuduPartialRow::*BinarySetter)(int, const Slice&);
hrm, would you mind removing the space between Status and the opening paren?
Also I'm not sure I fully understand how this typedef works but I'll ask you on
chat
Line 78: case COPY:
style nit: indent case per
https://google.github.io/styleguide/cppguide.html#Loops_and_Switch_Statements
Line 98: case COPY:
same
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3868/9/src/kudu/common/partial_row-test.cc
File src/kudu/common/partial_row-test.cc:
Line 47: typedef Status (KuduPartialRow::*BinarySetter)(int, const Slice&);
> hrm, would you mind removing the space between Status and the opening paren
OK I understand why this is needed now, based on our Slack conversation. Some
Googling on the topic led me to Stack Overflow where there is a lot of
discussion on std::function and overloaded methods.
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 9
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 10: Code-Review+1
(1 comment)
Looks good, just found one more doc nit
http://gerrit.cloudera.org:8080/#/c/3868/10/src/kudu/common/partial_row.h
File src/kudu/common/partial_row.h:
Line 236: /// These setters are same as the corresponding column-name-based
setters,
grammar nit: s/are same/are the same/ here and elsewhere
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 10
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has posted comments on this change.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 11
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: No
[kudu-CR] [KuduPartialRow::Set{Binary,String}()] copy input data
Mike Percy has submitted this change and it was merged.
Change subject: [KuduPartialRow::Set{Binary,String}()] copy input data
..
[KuduPartialRow::Set{Binary,String}()] copy input data
KuduPartialRow::SetBinary()/SetString() behavior was optimized
to omit copying of the passed data. However, a user of the API
might assume these methods are safe to use along with other setters
as SetInt32, SetDouble, etc. where the string or binary data
goes out of scope (or deallocated) before AppendToPB() is called.
To play safe, the behavior of these methods has been changed
to immediately copy the input data.
This is a safe modification, but it is not backward-compatible
in semver notation since it changes the semantics
of already existing methods. However, we don't bump API version
since Kudu is not 1.0 yet. As for the ABI, it remains compatible
with the prior versions: the existing client C++ code
will still compile and link.
This approach may add some performance regression issues for
already existing clients, but hopefully it is negligible for most
of the C++ clients around. As for the Impala-Kudu integration,
it seems current Impala code uses SetString() only in one place
at be/src/sec/kudu-table-sink.cc, and that can be addressed
separately.
An alternative approach might be to deprecate
KuduPartialRow::Set{Binary,String}() in favor of the newly introduced
KuduPartialRow::Set{Binary,String}NoCopy() methods. That approach
would spare us from performance regression worries and compatibility
issues. However, after some discussion, introducing the copying
behavior of KuduPartialRow::SetBinary()/SetString() methods
appeared to be more beneficial in the long run
from the usability perspective.
Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Reviewed-on: http://gerrit.cloudera.org:8080/3868
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy
---
M src/kudu/client/predicate-test.cc
M src/kudu/common/key_util-test.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/partial_row.cc
M src/kudu/common/partial_row.h
M src/kudu/common/row_operations-test.cc
M src/kudu/integration-tests/client_failover-itest.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/integration-tests/write_throttling-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tserver/tablet_copy_session-test.cc
12 files changed, 266 insertions(+), 72 deletions(-)
Approvals:
Mike Percy: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3868
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I764339a0e3ffbf6abd5372e682c9fa1792bdd52b
Gerrit-PatchSet: 12
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Dinesh Bhat
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Rename raft-remote-bootstrap design doc
Mike Percy has submitted this change and it was merged. Change subject: Rename raft-remote-bootstrap design doc .. Rename raft-remote-bootstrap design doc Change-Id: I1bf1de7edfd1412eb744f0c72c0e8c6db5491044 Reviewed-on: http://gerrit.cloudera.org:8080/3890 Tested-by: Kudu Jenkins Reviewed-by: Misty Stanley-Jones Reviewed-by: Mike Percy --- M docs/design-docs/README.md R docs/design-docs/raft-tablet-copy.md 2 files changed, 1 insertion(+), 1 deletion(-) Approvals: Misty Stanley-Jones: Looks good to me, approved Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1bf1de7edfd1412eb744f0c72c0e8c6db5491044 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR] Rename raft-remote-bootstrap design doc
Mike Percy has posted comments on this change. Change subject: Rename raft-remote-bootstrap design doc .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3890 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1bf1de7edfd1412eb744f0c72c0e8c6db5491044 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: No
[kudu-CR] Timestamp::FromUint64 should return void
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3943 to review the following change. Change subject: Timestamp::FromUint64 should return void .. Timestamp::FromUint64 should return void It does no validation, can't fail, and has the same signature as one of Timestamp's explicit constructors. Change-Id: I98d487abb7b9e947b6c8c0a291812f790407242e --- M src/kudu/common/timestamp.cc M src/kudu/common/timestamp.h M src/kudu/tablet/delta_stats.cc M src/kudu/tablet/tablet_bootstrap.cc 4 files changed, 5 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/3943/1 -- To view, visit http://gerrit.cloudera.org:8080/3943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I98d487abb7b9e947b6c8c0a291812f790407242e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] WIP: KUDU-236. Implement tablet history GC
Hello Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#5). Change subject: WIP: KUDU-236. Implement tablet history GC .. WIP: KUDU-236. Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Tests for Undo GC via major delta compaction * Test for entire-row GC via merging compaction TODO in this patch: 1. Determine if major delta compaction should be able to remove DELETE UNDO records, since 2 tests currently rely on that occurring, but they currently fail. 2. Need test for opening scanner at TS=0 and rejecting that scanner at the RPC level. Missing features (slated for future commit): * No dedicated background task that only schedules and performs history GC Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 22 files changed, 584 insertions(+), 135 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/5 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Clarify that minor delta compaction is for REDOs
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3944 to review the following change. Change subject: Clarify that minor delta compaction is for REDOs .. Clarify that minor delta compaction is for REDOs This just clarifies docs and comments around minor delta compaction, as it relates to the current implementation. Change-Id: I6e7f3822f3216c53818c082a03f1a1e5c98639d8 --- M docs/design-docs/tablet.md M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/tablet.h 5 files changed, 13 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/3944/1 -- To view, visit http://gerrit.cloudera.org:8080/3944 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6e7f3822f3216c53818c082a03f1a1e5c98639d8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Timestamp::FromUint64 should return void
Mike Percy has posted comments on this change. Change subject: Timestamp::FromUint64 should return void .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3943/1/src/kudu/common/timestamp.h File src/kudu/common/timestamp.h: Line 61: void FromUint64(uint64_t value); > hrm, maybe better to just remove this and use: ts = Timestamp(value) instea I don't know if it helps to not provide this mutation method because we also have bool DecodeFrom(Slice *input), as well as symmetrical methods ToUint64() and EncodeTo(faststring* s) -- To view, visit http://gerrit.cloudera.org:8080/3943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I98d487abb7b9e947b6c8c0a291812f790407242e Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Clarify that delta compaction is for REDOs
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3944 to look at the new patch set (#2). Change subject: Clarify that delta compaction is for REDOs .. Clarify that delta compaction is for REDOs This just clarifies docs and comments around delta compaction, as it relates to the current implementation. Change-Id: I6e7f3822f3216c53818c082a03f1a1e5c98639d8 --- M docs/design-docs/tablet.md M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/tablet.h 5 files changed, 24 insertions(+), 18 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/3944/2 -- To view, visit http://gerrit.cloudera.org:8080/3944 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7f3822f3216c53818c082a03f1a1e5c98639d8 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Clarify that minor delta compaction is for REDOs
Mike Percy has posted comments on this change. Change subject: Clarify that minor delta compaction is for REDOs .. Patch Set 1: (1 comment) actually i have a couple more changes i want to make http://gerrit.cloudera.org:8080/#/c/3944/1/src/kudu/tablet/tablet.h File src/kudu/tablet/tablet.h: Line 286: // issues a delta compaction. > why did you remove 'minor' here? Because the method takes a parameter indicating the type of compaction (minor or major) -- To view, visit http://gerrit.cloudera.org:8080/3944 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6e7f3822f3216c53818c082a03f1a1e5c98639d8 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] C++ client: deprecating KuduPartialRow::SetString()
Mike Percy has posted comments on this change. Change subject: C++ client: deprecating KuduPartialRow::SetString() .. Patch Set 12: Alexey: Feel free to abandon this. The alternative patch was merged: https://gerrit.cloudera.org/#/c/3868/ -- To view, visit http://gerrit.cloudera.org:8080/3723 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I348b0b9437b8d7928e3b607a0e0610d8d0c58f7c Gerrit-PatchSet: 12 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Dinesh Bhat Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Clarify that delta compaction is for REDOs
Mike Percy has submitted this change and it was merged. Change subject: Clarify that delta compaction is for REDOs .. Clarify that delta compaction is for REDOs This just clarifies docs and comments around delta compaction, as it relates to the current implementation. Change-Id: I6e7f3822f3216c53818c082a03f1a1e5c98639d8 Reviewed-on: http://gerrit.cloudera.org:8080/3944 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M docs/design-docs/tablet.md M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/tablet.h 5 files changed, 24 insertions(+), 18 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3944 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6e7f3822f3216c53818c082a03f1a1e5c98639d8 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Timestamp::FromUint64 should return void
Mike Percy has submitted this change and it was merged. Change subject: Timestamp::FromUint64 should return void .. Timestamp::FromUint64 should return void It does no validation, can't fail, and has the same signature as one of Timestamp's explicit constructors. Change-Id: I98d487abb7b9e947b6c8c0a291812f790407242e Reviewed-on: http://gerrit.cloudera.org:8080/3943 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/common/timestamp.cc M src/kudu/common/timestamp.h M src/kudu/tablet/delta_stats.cc M src/kudu/tablet/tablet_bootstrap.cc 4 files changed, 5 insertions(+), 7 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3943 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I98d487abb7b9e947b6c8c0a291812f790407242e Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR](gh-pages) www: Add Spark Summit EU talk
Mike Percy has posted comments on this change. Change subject: www: Add Spark Summit EU talk .. Patch Set 1: Code-Review+2 Verified+1 Tested, looks good -- To view, visit http://gerrit.cloudera.org:8080/3949 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I575c1ee7219a96a3c019cb2fe0899dfeef66 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR](gh-pages) www: Add Spark Summit EU talk
Mike Percy has submitted this change and it was merged. Change subject: www: Add Spark Summit EU talk .. www: Add Spark Summit EU talk Change-Id: I575c1ee7219a96a3c019cb2fe0899dfeef66 Reviewed-on: http://gerrit.cloudera.org:8080/3949 Reviewed-by: Mike Percy Tested-by: Mike Percy --- M community.md 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: Mike Percy: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/3949 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I575c1ee7219a96a3c019cb2fe0899dfeef66 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy Gerrit-Reviewer: Mike Percy
[kudu-CR] KUDU-564 (part 1): log a 'diff' when tablet config changes
Mike Percy has posted comments on this change. Change subject: KUDU-564 (part 1): log a 'diff' when tablet config changes .. Patch Set 1: > A tiny nit: is it possible to re-format the commit message to > preserve the useful information but still fit into 72 chars? +1 on Todd's 80 character wrapping in Git. Git with the program! However not sure about wrapping that info message. I don't think it's worth it. -- To view, visit http://gerrit.cloudera.org:8080/3939 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb785093176eea067de039b14f6600084998861a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-564 (part 1): log a 'diff' when tablet config changes
Mike Percy has posted comments on this change. Change subject: KUDU-564 (part 1): log a 'diff' when tablet config changes .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3939 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb785093176eea067de039b14f6600084998861a Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1555. PBC Flush() method should be async
Mike Percy has posted comments on this change. Change subject: KUDU-1555. PBC Flush() method should be async .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-1555. PBC Flush() method should be async
Mike Percy has posted comments on this change. Change subject: KUDU-1555. PBC Flush() method should be async .. Patch Set 2: Wow, wish we had noticed this earlier -- To view, visit http://gerrit.cloudera.org:8080/3951 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I721707070fe47e3377d791c95214f007c90d2263 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-236. Implement tablet history GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#7). Change subject: KUDU-236. Implement tablet history GC .. KUDU-236. Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Test for Redo GC via major delta compaction * Test for history GC via tablet flush * Test for Undo GC via merge compaction * Test for entire-row GC via merge compaction TODO: Tests still needed: * Try to generate empty files (UNDO, REDO) * Test for opening scanner at TS < AHM and rejecting that scanner at the RPC level. Missing features (slated for future commit): * No dedicated background task that only schedules and performs undo GC Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 22 files changed, 659 insertions(+), 140 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/7 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-236. Implement tablet history GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#6). Change subject: KUDU-236. Implement tablet history GC .. KUDU-236. Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Test for Redo GC via major delta compaction * Test for history GC via tablet flush * Test for Undo GC via merge compaction * Test for entire-row GC via merge compaction TODO: Tests still needed: * Try to generate empty files (UNDO, REDO) * Test for opening scanner at TS < AHM and rejecting that scanner at the RPC level. Missing features (slated for future commit): * No dedicated background task that only schedules and performs undo GC Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 22 files changed, 649 insertions(+), 136 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/6 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-564 (part 1): log a 'diff' when tablet config changes
Mike Percy has posted comments on this change. Change subject: KUDU-564 (part 1): log a 'diff' when tablet config changes .. Patch Set 1: Code-Review+2 (1 comment) Nice. Looks good except for one thing I was wondering about http://gerrit.cloudera.org:8080/#/c/3939/1/src/kudu/consensus/quorum_util.cc File src/kudu/consensus/quorum_util.cc: Line 255: peer_infos[old_state.leader_uuid()].first.last_known_addr().host(), not guaranteed to have a leader so i am not sure what happens when last_known_addr is empty. Is it friendly and we just end up with an empty string from host()? Maybe we want a somewhat nicer message in that case. -- To view, visit http://gerrit.cloudera.org:8080/3939 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb785093176eea067de039b14f6600084998861a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: Yes
[kudu-CR](gh-pages) www: Add Spark Summit EU talk
Mike Percy has uploaded a new change for review. http://gerrit.cloudera.org:8080/3949 Change subject: www: Add Spark Summit EU talk .. www: Add Spark Summit EU talk Change-Id: I575c1ee7219a96a3c019cb2fe0899dfeef66 --- M community.md 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/3949/1 -- To view, visit http://gerrit.cloudera.org:8080/3949 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I575c1ee7219a96a3c019cb2fe0899dfeef66 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: gh-pages Gerrit-Owner: Mike Percy
[kudu-CR] Add Apache Kudu logo to the source tree
Mike Percy has abandoned this change. Change subject: Add Apache Kudu logo to the source tree .. Abandoned We'll try to find another place for this. -- To view, visit http://gerrit.cloudera.org:8080/3701 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I4ab7185407e23c1c6c2620723ca44910248b6462 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add regex string match assert helper macros
Mike Percy has uploaded a new change for review. http://gerrit.cloudera.org:8080/3962 Change subject: Add regex string match assert helper macros .. Add regex string match assert helper macros Change-Id: I05d869479f4032d584a83b455979073c7707d9e7 --- M src/kudu/util/test_macros.h 1 file changed, 54 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/3962/1 -- To view, visit http://gerrit.cloudera.org:8080/3962 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I05d869479f4032d584a83b455979073c7707d9e7 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy
[kudu-CR] KUDU-236. Implement tablet history GC
Mike Percy has posted comments on this change. Change subject: KUDU-236. Implement tablet history GC .. Patch Set 6: (11 comments) Posting partially addressed feedback except for 2 comments in compaction.cc and 2 comments in tablet_history_gc-test.cc http://gerrit.cloudera.org:8080/#/c/3076/6/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: Line 835: num_rows_history_truncated += is_history_truncated; > we lost the nice WARNING we used to have for this case which actually inclu Fixed http://gerrit.cloudera.org:8080/#/c/3076/6/src/kudu/tablet/compaction.h File src/kudu/tablet/compaction.h: Line 42: HistoryGcOpts(GcHistoryEnabled is_enabled, Timestamp ahm) > I don't think this ctor buys much over just using brace initialization like Done http://gerrit.cloudera.org:8080/#/c/3076/6/src/kudu/tablet/delta_compaction.cc File src/kudu/tablet/delta_compaction.cc: Line 150: VLOG(2) << "Input Row: " << dst_row.schema()->DebugRow(dst_row) << > this should probably be a much higher VLOG level (and maybe DVLOG) Done Line 191: //TODO: Why keep the delete mutations? > because major delta compaction doesn't change row IDs, and thus you can't a Removed comment http://gerrit.cloudera.org:8080/#/c/3076/6/src/kudu/tablet/tablet-harness.h File src/kudu/tablet/tablet-harness.h: Line 112: clock_ = make_scoped_refptr(new server::HybridClock()); > do you need make_scoped_refptr here? given clock_ is a scoped_refptr I thin Changed to reset() which is syntax that makes it more clear that we're giving ownership to a smart ptr. http://gerrit.cloudera.org:8080/#/c/3076/6/src/kudu/tablet/tablet.cc File src/kudu/tablet/tablet.cc: Line 111: DEFINE_int32(tablet_history_max_age_sec, 604800, > can you express this as 60 * 60 * 24 * 7 so it's obvious it's 7 days? I'll just set it to -1. Line 114: "To disable history removal, set to -1."); > I think we should probably disable this for the first commit, until we can Done http://gerrit.cloudera.org:8080/#/c/3076/6/src/kudu/tablet/tablet_history_gc-test.cc File src/kudu/tablet/tablet_history_gc-test.cc: PS6, Line 73: ws > nit: capitalize RowSet Done PS6, Line 118: Major delta compaction will not remove UNDOs, so we cannot read : // from before the initial insert of the data and expect to see base data. > not following this part of the comment -- you haven't run the MajorDeltaCom Done Line 134: // equal to 2. > I guess the comment above really applies here: you're saying that if you re > I guess the comment above really applies here: you're saying that if you read > from prior to the insertion, you'll still see the rows disappear because the > UNDOs weren't removed, right? Done > One suggestion to make these tests slightly easier to follow is to use > RowSet::DebugDump() which gives you a nice printout per row that you can > assert on. (though you'll have to set the hybrid timestamp to a constant > instead of Now()) Spent some time implementing this idea, got some decent results with regex matches. PS6, Line 169: No trace should remain after this > add an assertion that it actually didn't even generate a DRS Done -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-236. Implement tablet history GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#8). Change subject: KUDU-236. Implement tablet history GC .. KUDU-236. Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Test for Redo GC via major delta compaction * Test for history GC via tablet flush * Test for Undo GC via merge compaction * Test for entire-row GC via merge compaction * Test for opening a scanner at TS < AHM and rejecting that scanner at the RPC level. Missing features (slated for future commit): * No dedicated background task that only schedules and performs undo GC Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 24 files changed, 726 insertions(+), 135 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/8 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add regex string match assert helper macros
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3962 to look at the new patch set (#2). Change subject: Add regex string match assert helper macros .. Add regex string match assert helper macros These delegate to gmock but are easier to remember than: ASSERT_THAT(str, testing::ContainsRegex(pattern)) Change-Id: I05d869479f4032d584a83b455979073c7707d9e7 --- M src/kudu/util/test_macros.h 1 file changed, 41 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/3962/2 -- To view, visit http://gerrit.cloudera.org:8080/3962 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I05d869479f4032d584a83b455979073c7707d9e7 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-236. Implement tablet history GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#9). Change subject: KUDU-236. Implement tablet history GC .. KUDU-236. Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Test for Redo GC via major delta compaction * Test for history GC via tablet flush * Test for Undo GC via merge compaction * Test for entire-row GC via merge compaction * Test for opening a scanner at TS < AHM and rejecting that scanner at the RPC level. Missing features (slated for future commit): * No dedicated background task that only schedules and performs undo GC Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 24 files changed, 726 insertions(+), 135 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/9 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
