[kudu-CR] KUDU-2279 (part 5): don't include entity attributes in log
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9180 ) Change subject: KUDU-2279 (part 5): don't include entity attributes in log .. Patch Set 3: Code-Review+2 Carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/9180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic995f6a3b8430b73a041de8e5bcbc53a5d8e7f1b Gerrit-Change-Number: 9180 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Feb 2018 07:59:12 + Gerrit-HasComments: No
[kudu-CR] KUDU-2279 (part 3): metrics: don't emit untouched metrics to log
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9177 ) Change subject: KUDU-2279 (part 3): metrics: don't emit untouched metrics to log .. KUDU-2279 (part 3): metrics: don't emit untouched metrics to log This avoids a big dump of zero-valued metrics at startup. Change-Id: I92d2c4640e54c91791fab9c420215bafa3fe8f20 Reviewed-on: http://gerrit.cloudera.org:8080/9177 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/server/server_base.cc M src/kudu/util/metrics-test.cc M src/kudu/util/metrics.cc M src/kudu/util/metrics.h 4 files changed, 68 insertions(+), 3 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I92d2c4640e54c91791fab9c420215bafa3fe8f20 Gerrit-Change-Number: 9177 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2279 (part 4): enable metrics log by default
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9178 ) Change subject: KUDU-2279 (part 4): enable metrics log by default .. KUDU-2279 (part 4): enable metrics log by default Change-Id: Iaeb4c546c2800d8f4288a3626b9e998cd7e35e5c Reviewed-on: http://gerrit.cloudera.org:8080/9178 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/server/server_base_options.cc 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iaeb4c546c2800d8f4288a3626b9e998cd7e35e5c Gerrit-Change-Number: 9178 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9176 ) Change subject: KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log .. KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log This adds a global 'metrics epoch' which can be externally incremented. When metrics are modified, they remember the epoch of their most recent modification. When we dump metrics, we can pass a lower bound in order to see only metrics which have been modified in or after a given epoch. This patch updates the metrics logging to only emit metrics that have changed in each successive line. This should substantially reduce the size and CPU cost of metric logging on servers with thousands of tablets. Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e Reviewed-on: http://gerrit.cloudera.org:8080/9176 Tested-by: Kudu Jenkins Reviewed-by: Will BerkeleyReviewed-by: Mike Percy --- M src/kudu/server/server_base.cc M src/kudu/util/metrics-test.cc M src/kudu/util/metrics.cc M src/kudu/util/metrics.h 4 files changed, 141 insertions(+), 26 deletions(-) Approvals: Kudu Jenkins: Verified Will Berkeley: Looks good to me, but someone else must approve Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e Gerrit-Change-Number: 9176 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2279 (part 4): enable metrics log by default
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9178 ) Change subject: KUDU-2279 (part 4): enable metrics log by default .. Patch Set 3: Code-Review+2 carry +2 -- To view, visit http://gerrit.cloudera.org:8080/9178 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaeb4c546c2800d8f4288a3626b9e998cd7e35e5c Gerrit-Change-Number: 9178 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Feb 2018 07:58:50 + Gerrit-HasComments: No
[kudu-CR] KUDU-2279 (part 3): metrics: don't emit untouched metrics to log
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9177 ) Change subject: KUDU-2279 (part 3): metrics: don't emit untouched metrics to log .. Patch Set 3: Code-Review+2 carrying +2 -- To view, visit http://gerrit.cloudera.org:8080/9177 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I92d2c4640e54c91791fab9c420215bafa3fe8f20 Gerrit-Change-Number: 9177 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Feb 2018 07:58:38 + Gerrit-HasComments: No
[kudu-CR] KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9176 ) Change subject: KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log .. Patch Set 3: Yea, though we do dump all of the entities even if none of their metrics changed so we'll see them disappear when retired. -- To view, visit http://gerrit.cloudera.org:8080/9176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e Gerrit-Change-Number: 9176 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Feb 2018 07:58:16 + Gerrit-HasComments: No
[kudu-CR] KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9176 ) Change subject: KUDU-2279 (part 2): metrics: only emit changed metrics in metrics log .. Patch Set 3: Code-Review+2 Nice feature. One downside is that it's not possible to tell the difference between metrics that were retired (tombstoned tablets) and metrics that haven't changed, but in most cases we probably won't care. -- To view, visit http://gerrit.cloudera.org:8080/9176 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia26be99a1fa96d52e2ca0905844d56c096d3778e Gerrit-Change-Number: 9176 Gerrit-PatchSet: 3 Gerrit-Owner: Todd LipconGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Feb 2018 07:36:59 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/8692 ) Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client .. Patch Set 10: Code-Review+1 (2 comments) This LGTM http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc File src/kudu/hms/sasl_client_transport.cc: http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc@123 PS5, Line 123: ead > It's probably not possible currently, but exception handling is non-local, Ok that makes sense. I'm fine with keeping it this way too. The reason I brought it up was because if some unexpected exception was thrown, we'd ideally want to fix it rather than transparently drop it. But I'm fine with it either way. http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc File src/kudu/hms/sasl_client_transport.cc: http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc@205 PS7, Line 205: > Yes, the 'ResetWriteBuf' method restores the prefix, and that's called at t Oops, missed that. Thanks for the explanation. -- To view, visit http://gerrit.cloudera.org:8080/8692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 Gerrit-Change-Number: 8692 Gerrit-PatchSet: 10 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Feb 2018 04:34:17 + Gerrit-HasComments: Yes
[kudu-CR] WIP: Create a rolling-failure endurance test
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9222 to look at the new patch set (#3). Change subject: WIP: Create a rolling-failure endurance test .. WIP: Create a rolling-failure endurance test This script is designed to run for an extended period of time on a cluster and cause a rolling failure and then restart of all the kudu-tserver daemons on the cluster. After killing all of the kudu-tserver processes in random order on a set of tablet servers, the script will restart all of the processes and then run ksck until the cluster is healthy again. The script assumes that it has passwordless sudo access on the local machine, that the "kudu" administrative tool is in the local PATH, that it has passwordless sudo access on every machine on the cluster (after ssh'ing into each machine), and that the starting state is a healthy cluster with all daemons running. Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc --- A src/kudu/scripts/rolling-failure-test.pl 1 file changed, 258 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/9222/3 -- To view, visit http://gerrit.cloudera.org:8080/9222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc Gerrit-Change-Number: 9222 Gerrit-PatchSet: 3 Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP: Create a rolling-failure endurance test
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9222 to look at the new patch set (#2). Change subject: WIP: Create a rolling-failure endurance test .. WIP: Create a rolling-failure endurance test This script is designed to run for an extended period of time on a cluster and cause a rolling failure and then restart of all the kudu-tserver daemons on the cluster. After killing all of the kudu-tserver processes in random order on a set of tablet servers, the script will restart all of the processes and then run ksck until the cluster is healthy again. The script assumes that it has passwordless sudo access on the local machine, that the "kudu" administrative tool is in the local PATH, that it has passwordless sudo access on every machine on the cluster (after ssh'ing into each machine), and that the starting state is a healthy cluster with all daemons running. Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc --- A src/kudu/scripts/rolling-failure-test.pl 1 file changed, 258 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/9222/2 -- To view, visit http://gerrit.cloudera.org:8080/9222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc Gerrit-Change-Number: 9222 Gerrit-PatchSet: 2 Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] WIP: Create a rolling-failure endurance test
Hello Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9222 to review the following change. Change subject: WIP: Create a rolling-failure endurance test .. WIP: Create a rolling-failure endurance test This script is designed to run for an extended period of time on a cluster and cause a rolling failure and then restart of all the kudu-tserver daemons on the cluster. After killing all of the kudu-tserver processes in random order on a set of tablet servers, the script will restart all of the processes and then run ksck until the cluster is healthy again. The script assumes that it has passwordless sudo access on the local machine, that the "kudu" administrative tool is in the local PATH, that it has passwordless sudo access on every machine on the cluster (after ssh'ing into each machine), and that the starting state is a healthy cluster with all daemons running. Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc --- A src/kudu/scripts/rolling-failure-test.pl 1 file changed, 0 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/22/9222/1 -- To view, visit http://gerrit.cloudera.org:8080/9222 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib47547f06c0f490a1798990b46d3156c583320fc Gerrit-Change-Number: 9222 Gerrit-PatchSet: 1 Gerrit-Owner: Mike PercyGerrit-Reviewer: Alexey Serbin
[kudu-CR] build-support: fix reporting of build configuration
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9182 ) Change subject: build-support: fix reporting of build configuration .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6c131f46e75d2eb57d7e0ce9d2ad21e2ad9ab80 Gerrit-Change-Number: 9182 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Tue, 06 Feb 2018 03:24:34 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Sailesh Mukil, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8692 to look at the new patch set (#10). Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client .. KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client The bulk of this commit is adding a new Thrift transport type, SaslClientTransport, which facilitates SASL GSSAPI negotiation, as well as integrity/privacy channel protection. The new transport is based on Impala's version with some significant changes: - Impala has a client and server SASL transport, necessitating a common superclass (SaslTransport). Since we only need a client transport, I collapsed all of the logic into a single class, which I think makes the code easier to follow. - The transport uses Kudu helper types where possible, e.g., faststring buffers, and our existing SASL utility infrastructure. - Integrity and privacy channel protection are implemented. There are no standlone unit-tests for the transport, since that would require implementing the server-specific counterpart. Instead, the class is tested indirectly through using the HMS client to communicate with a Kerberos-enabled HMS instance. Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 --- M src/kudu/hms/CMakeLists.txt M src/kudu/hms/hms_client-test.cc M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h A src/kudu/hms/sasl_client_transport.cc A src/kudu/hms/sasl_client_transport.h M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/sasl_common.cc M src/kudu/rpc/sasl_common.h M src/kudu/rpc/server_negotiation.cc 14 files changed, 994 insertions(+), 83 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/8692/10 -- To view, visit http://gerrit.cloudera.org:8080/8692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 Gerrit-Change-Number: 8692 Gerrit-PatchSet: 10 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tablet: don't store row count in delta tracker
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9216 ) Change subject: tablet: don't store row count in delta tracker .. Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h File src/kudu/tablet/diskrowset.h: http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@366 PS5, Line 366: Status CountRows(rowid_t *count) const override; can we mark this final so that calls from within this class can be inlined instead of virtual calls? http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@474 PS5, Line 474: // Number of rows in the rowset. can you add a comment here that this might be unset? http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.h@475 PS5, Line 475: mutable std::atomic num_rows_; let's just use int64_t here, rowid_t is kinda old and gross http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@522 PS5, Line 522: num_rows_(0), let's use -1 as a signifier of unknown here http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@672 PS5, Line 672: rowid_t num_rows; : RETURN_NOT_OK(CountRows(_rows)); this can be inside an #ifndef NDEBUG right? http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@700 PS5, Line 700: rowid_t num_rows; same http://gerrit.cloudera.org:8080/#/c/9216/5/src/kudu/tablet/diskrowset.cc@725 PS5, Line 725: { I don't think the extra nesting is worth making the critical section one store shorter -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew WongGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Feb 2018 01:47:29 + Gerrit-HasComments: Yes
[kudu-CR] Improved logging of DMS flushes
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9204 ) Change subject: Improved logging of DMS flushes .. Improved logging of DMS flushes >From compaction-test.cc's TestRowSetInput, Before: I0202 14:52:24.983525 2427953984 delta_tracker.cc:727] T test_tablet_id P a3e9bdf25f0b431ea57d0249535c29b3: Flushing 20 deltas from DMS 0... I0202 14:52:24.984046 2427953984 delta_tracker.cc:668] T test_tablet_id P a3e9bdf25f0b431ea57d0249535c29b3: Flushed delta block: 0198399763226209 ts range: [11, 30] After: I0202 14:45:18.924826 2427953984 delta_tracker.cc:726] T test_tablet_id P 37440651650a4d858267ad408b7e1ac0: Flushing 20 deltas (1759 bytes on disk) from DMS 0 I0202 14:45:18.925750 2427953984 delta_tracker.cc:668] T test_tablet_id P 37440651650a4d858267ad408b7e1ac0: Flushed delta block 0052242700095437 (296 bytes in memory) stats: ts range=[11, 30], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[12:20,11:20] Plus one tiny change to clarify logging on flushes. Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65 Reviewed-on: http://gerrit.cloudera.org:8080/9204 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon--- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/tablet.cc 3 files changed, 11 insertions(+), 6 deletions(-) Approvals: Kudu Jenkins: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65 Gerrit-Change-Number: 9204 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] Improved logging of DMS flushes
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9204 ) Change subject: Improved logging of DMS flushes .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65 Gerrit-Change-Number: 9204 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Tue, 06 Feb 2018 01:40:34 + Gerrit-HasComments: No
[kudu-CR] KUDU-1704: add READ YOUR WRITES scan mode
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/8804 ) Change subject: KUDU-1704: add READ_YOUR_WRITES scan mode .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/8804/10/src/kudu/tserver/tablet_server-test.cc@1953 PS10, Line 1953: // Make sure that there is no snapshot timestamp sent back. : ASSERT_TRUE(!resp.has_snap_timestamp()); > I'm still unsure why we wouldn't the response to include the timestamp. I a I see, so looks like you are proposing to make this read mode fault-tolerant as well. Which means we need to make it an ordered scan and ensure the client sent out the last primary key to retry on. I do not see any reasons to not make this mode fault tolerant, but I do not see the reasons why we have to return the chosen snapshot timestamp for the scan to retry on another server in this case. I think the next tsever that the scan is retried on can chose the timestamp freely based on the conditions, right? Moreover, would you mind I do another patch if we do want RYW to be fault tolerant, to make it more clear? -- To view, visit http://gerrit.cloudera.org:8080/8804 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I84ddb981a1a0f199d4e66f5d5097318f8c785a48 Gerrit-Change-Number: 8804 Gerrit-PatchSet: 10 Gerrit-Owner: Hao HaoGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Comment-Date: Tue, 06 Feb 2018 01:21:14 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Sailesh Mukil, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8692 to look at the new patch set (#9). Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client .. KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client The bulk of this commit is adding a new Thrift transport type, SaslClientTransport, which facilitates SASL GSSAPI negotiation, as well as integrity/privacy channel protection. The new transport is based on Impala's version with some significant changes: - Impala has a client and server SASL transport, necessitating a common superclass (SaslTransport). Since we only need a client transport, I collapsed all of the logic into a single class, which I think makes the code easier to follow. - The transport uses Kudu helper types where possible, e.g., faststring buffers, and our existing SASL utility infrastructure. - Integrity and privacy channel protection are implemented. There are no standlone unit-tests for the transport, since that would require implementing the server-specific counterpart. Instead, the class is tested indirectly through using the HMS client to communicate with a Kerberos-enabled HMS instance. Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 --- M src/kudu/hms/CMakeLists.txt M src/kudu/hms/hms_client-test.cc M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h A src/kudu/hms/sasl_client_transport.cc A src/kudu/hms/sasl_client_transport.h M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/sasl_common.cc M src/kudu/rpc/sasl_common.h M src/kudu/rpc/server_negotiation.cc 14 files changed, 995 insertions(+), 84 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/8692/9 -- To view, visit http://gerrit.cloudera.org:8080/8692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 Gerrit-Change-Number: 8692 Gerrit-PatchSet: 9 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8692 ) Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client .. Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc File src/kudu/hms/sasl_client_transport.cc: http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc@123 PS5, Line 123: ans > I didn't realize we could hit non Thrift exceptions in Negotiate(), since S It's probably not possible currently, but exception handling is non-local, and this is more robust to future changes. Is there a reason to prefer scoping down the catch? http://gerrit.cloudera.org:8080/#/c/8692/5/src/kudu/hms/sasl_client_transport.cc@274 PS5, Line 274: > nvm, I misunderstood the fact that quth-int and auth-conf need to be negoti I ended up changing the API so that the minimum protection level is passed in explicitly; hopefully this makes it a bit more clear (and it reduces some code). http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc File src/kudu/hms/sasl_client_transport.cc: http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc@141 PS7, Line 141: read_buf_.resize(kFrameHeaderSize); : transport_->readAll(read_buf_.data(), kFrameHeaderSize); > Why not just read into a uint32_t here and avoid the resize(kFrameHeaderSiz Done http://gerrit.cloudera.org:8080/#/c/8692/7/src/kudu/hms/sasl_client_transport.cc@205 PS7, Line 205: plaintext.remove_prefix(kFrameHeaderSize); > Is this correct? Yes, the 'ResetWriteBuf' method restores the prefix, and that's called at the very end of flush, so it should be present on subsequent trips through the loop. -- To view, visit http://gerrit.cloudera.org:8080/8692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 Gerrit-Change-Number: 8692 Gerrit-PatchSet: 7 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Feb 2018 00:58:53 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/8692 ) Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client .. Patch Set 7: Todd had some offline feedback to give the receive buffer size a flag, which necessitated a few refactors, so the latest diff is kinda big. -- To view, visit http://gerrit.cloudera.org:8080/8692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 Gerrit-Change-Number: 8692 Gerrit-PatchSet: 7 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 06 Feb 2018 00:59:41 + Gerrit-HasComments: No
[kudu-CR] KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Sailesh Mukil, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8692 to look at the new patch set (#8). Change subject: KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client .. KUDU-2191 (5/n): Add Kerberos SASL support to the HMS client The bulk of this commit is adding a new Thrift transport type, SaslClientTransport, which facilitates SASL GSSAPI negotiation, as well as integrity/privacy channel protection. The new transport is based on Impala's version with some significant changes: - Impala has a client and server SASL transport, necessitating a common superclass (SaslTransport). Since we only need a client transport, I collapsed all of the logic into a single class, which I think makes the code easier to follow. - The transport uses Kudu helper types where possible, e.g., faststring buffers, and our existing SASL utility infrastructure. - Integrity and privacy channel protection are implemented. There are no standlone unit-tests for the transport, since that would require implementing the server-specific counterpart. Instead, the class is tested indirectly through using the HMS client to communicate with a Kerberos-enabled HMS instance. Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 --- M src/kudu/hms/CMakeLists.txt M src/kudu/hms/hms_client-test.cc M src/kudu/hms/hms_client.cc M src/kudu/hms/hms_client.h M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h A src/kudu/hms/sasl_client_transport.cc A src/kudu/hms/sasl_client_transport.h M src/kudu/mini-cluster/external_mini_cluster-test.cc M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/sasl_common.cc M src/kudu/rpc/sasl_common.h M src/kudu/rpc/server_negotiation.cc 14 files changed, 987 insertions(+), 84 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/92/8692/8 -- To view, visit http://gerrit.cloudera.org:8080/8692 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8f217ae05fd36c8ee88fe20eeccd73d49233a345 Gerrit-Change-Number: 8692 Gerrit-PatchSet: 8 Gerrit-Owner: Dan BurkertGerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tablet: don't store row count in delta tracker
Hello Tidy Bot, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9216 to look at the new patch set (#5). Change subject: tablet: don't store row count in delta tracker .. tablet: don't store row count in delta tracker DeltaTracker's constructor previously required a row-count that can only be obtained by first reading from disk. While useful as a sanity check while tracking updates, it's not important for this count to be known at instantiation-time. This patch is helpful in reducing the amount of data required from disk at startup, as one of the reasons we need to fully open some CFiles is to get this row count. This patch itself doesn't defer any reads; it just defers the requirement of the row count. This patch has no functional changes. Change-Id: I084e0944f388c22e838b017663a812b0ba77245d --- M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h 5 files changed, 41 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/9216/5 -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew WongGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure
Mahdi Askari has posted comments on this change. ( http://gerrit.cloudera.org:8080/8960 ) Change subject: Adding a subsection to NTP troubleshooting section to cover frequent cases of NTP crashes when the Kudu cluster is deployed on Microsoft Azure .. Patch Set 3: (7 comments) Comments replied. Please review. http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc File docs/troubleshooting.adoc: http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@125 PS1, Line 125: NTP Clock on secure clusters with Centrify > nit: trailing whitespace Done http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@127 PS1, Line 127: - CentrifyDC (Linux package which connects the host to an Active Directory) bypasses NTP program. > nit: can you wrap this to 80-100 coluns as done elsewhere in the file? Done http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: > can you format file names appropriately using `...`? Done http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: > this raw link sitting in the documentation doesn't look good Done http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: > typo Done http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@129 PS1, Line 129: nk:https:/ > is Centrify Azure-specific? Should this be a separate troubleshooting point It is not Azure Specific. Can be any cloud or on-prem. I will move it to its own section. http://gerrit.cloudera.org:8080/#/c/8960/1/docs/troubleshooting.adoc@135 PS1, Line 135: > how can this script be automated to run at startup on every startup? Seems I will have a look and see how we can automate this in start up. -- To view, visit http://gerrit.cloudera.org:8080/8960 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d638aa26137d3a795fce1d0f3f744d4969bdf22 Gerrit-Change-Number: 8960 Gerrit-PatchSet: 3 Gerrit-Owner: Mahdi AskariGerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mahdi Askari Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Feb 2018 23:47:30 + Gerrit-HasComments: Yes
[kudu-CR] tablet: don't store row count in delta tracker
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9216 ) Change subject: tablet: don't store row count in delta tracker .. Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1191 PS2, Line 1191: disk rowsets th > update comment Done http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1290 PS2, Line 1290: RETURN_NOT_OK(cur_drs->CountRows(_rows)); > worried about the perf impact here -- this takes a lock and is un-inlinable Done http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@530 PS2, Line 530: > perhaps here we should cache num_rows_ in DiskRowSet Done http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@709 PS2, Line 709: } > same, this is a hot function Done -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew WongGerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Feb 2018 23:36:31 + Gerrit-HasComments: Yes
[kudu-CR] tablet: don't store row count in delta tracker
Hello Tidy Bot, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9216 to look at the new patch set (#4). Change subject: tablet: don't store row count in delta tracker .. tablet: don't store row count in delta tracker DeltaTracker's constructor previously required a row-count that can only be obtained by first reading from disk. While useful as a sanity check while tracking updates, it's not important for this count to be known at instantiation-time. This patch is helpful in reducing the amount of data required from disk at startup, as one of the reasons we need to fully open some CFiles is to get this row count. This patch itself doesn't defer any reads, it just defers the requirement of the row count. This patch has no functional changes. Change-Id: I084e0944f388c22e838b017663a812b0ba77245d --- M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h 5 files changed, 38 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/9216/4 -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew WongGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tablet: don't store row count in delta tracker
Hello Tidy Bot, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9216 to look at the new patch set (#3). Change subject: tablet: don't store row count in delta tracker .. tablet: don't store row count in delta tracker DeltaTracker's constructor previously required a row-count that can only be obtained by first reading from disk. While useful as a sanity check while tracking updates, it's not important for this count to be known at instantiation-time. This patch is helpful in reducing the amount of data required from disk at startup, as one of the reasons we need to fully open some CFiles is to get this row count. This patch itself doesn't defer any reads, it just defers the requirement of the row count. This patch has no functional changes. Change-Id: I084e0944f388c22e838b017663a812b0ba77245d --- M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h 5 files changed, 38 insertions(+), 36 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/9216/3 -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew WongGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tablet: don't store row count in delta tracker
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9216 ) Change subject: tablet: don't store row count in delta tracker .. Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1191 PS2, Line 1191: delta trackers update comment http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/compaction.cc@1290 PS2, Line 1290: RETURN_NOT_OK(cur_drs->CountRows(_rows)); worried about the perf impact here -- this takes a lock and is un-inlinable, and this is the hot path of a compaction. Can we cache the row count somewhere? http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc File src/kudu/tablet/diskrowset.cc: http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@530 PS2, Line 530: RETURN_NOT_OK(DeltaTracker::Open(rowset_metadata_, perhaps here we should cache num_rows_ in DiskRowSet http://gerrit.cloudera.org:8080/#/c/9216/2/src/kudu/tablet/diskrowset.cc@709 PS2, Line 709: DCHECK_LT(row_idx, num_rows); same, this is a hot function -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew WongGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Feb 2018 21:46:10 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9199 ) Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. KUDU-2281: Add Primary Key Support For INT128 Columns Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Reviewed-on: http://gerrit.cloudera.org:8080/9199 Tested-by: Kudu Jenkins Reviewed-by: Grant Henke--- M src/kudu/client/predicate-test.cc M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/common/encoded_key-test.cc M src/kudu/common/key_encoder.cc M src/kudu/common/key_encoder.h M src/kudu/common/key_util-test.cc M src/kudu/common/key_util.cc M src/kudu/common/partial_row.h M src/kudu/integration-tests/all_types-itest.cc 10 files changed, 128 insertions(+), 16 deletions(-) Approvals: Kudu Jenkins: Verified Grant Henke: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9199 ) Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. Patch Set 6: Added back Dan's +2 after a negligible lint fix. -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 6 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 20:33:39 + Gerrit-HasComments: No
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9199 ) Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 6 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 20:33:17 + Gerrit-HasComments: No
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9161 ) Change subject: Don't rely on pending config OpId index for peer promotion .. Don't rely on pending config OpId index for peer promotion As part of the effort to make OpId and timestamp assignment atomic we can't rely on a pending config having been assigned an OpId anymore. This because the config is marked as the active config _before_ an OpId is assigned later on, by the queue. The OpId only actually gets set when the config is committed. Follow up patches will complete remove this reliance. In general this doesn't have much impact except where we're promoting a peer. In this case we currently pass the current active config's term and index from the queue back to consensus which then uses this info to perform the promotion only when the current term and index match. We can't do this in the new setting where the term and index are not available, but moreover this info is not strictly needed for promotion at all, so this patch removes this extra info being passed around. The reasoning why the extra info is not needed is the following: - If a peer' uuid is marked for promotion in the current committed config, as verified directly through cmeta, and is considered up-to-date enough to be promoted, it doesn't matter whether the active config in the queue is the same as the committed config. The peer still can and should be promoted. - Essentially, omitting this extra info just means that we have to re-check that the member type and attributes are still the expected ones, which is not costly. Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761 Reviewed-on: http://gerrit.cloudera.org:8080/9161 Tested-by: Kudu Jenkins Reviewed-by: Alexey SerbinReviewed-by: Mike Percy --- M src/kudu/consensus/consensus_queue.cc M src/kudu/consensus/consensus_queue.h M src/kudu/consensus/raft_consensus.cc M src/kudu/consensus/raft_consensus.h 4 files changed, 34 insertions(+), 54 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, but someone else must approve Mike Percy: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761 Gerrit-Change-Number: 9161 Gerrit-PatchSet: 4 Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 ) Change subject: Don't rely on pending config OpId index for peer promotion .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761 Gerrit-Change-Number: 9161 Gerrit-PatchSet: 3 Gerrit-Owner: David Ribeiro AlvesGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 05 Feb 2018 19:49:29 + Gerrit-HasComments: No
[kudu-CR] tablet: don't store row count in delta tracker
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9216 to look at the new patch set (#2). Change subject: tablet: don't store row count in delta tracker .. tablet: don't store row count in delta tracker DeltaTracker's constructor previously required a row-count that can only be obtained by first reading from disk. While useful as a sanity check while tracking updates, it's not important for this count to be known at instantiation-time. This patch is helpful in reducing the amount of data required from disk at startup, as one of the reasons we need to fully open some CFiles is to get this row count. This patch itself doesn't defer any reads, it just defers the requirement of the row count. This patch has no functional changes. Change-Id: I084e0944f388c22e838b017663a812b0ba77245d --- M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc 4 files changed, 22 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/9216/2 -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 2 Gerrit-Owner: Andrew WongGerrit-Reviewer: Kudu Jenkins
[kudu-CR] tablet: don't store row count in delta tracker
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9216 Change subject: tablet: don't store row count in delta tracker .. tablet: don't store row count in delta tracker DeltaTracker's constructor previously required a row-count that can only be obtained by first reading its parent rowset's footer from disk. While useful as a sanity check while tracking updates, it's not important for this count to be known at initialization. This patch is helpful in reducing the amount of data required from disk at startup, as one of the reasons we need to fully open some CFiles is to get this row count. This patch itself doesn't defer any reads, it just defers the requirement of the row count. This patch has no functional changes. Change-Id: I084e0944f388c22e838b017663a812b0ba77245d --- M src/kudu/tablet/compaction.cc M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/diskrowset.cc 4 files changed, 22 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/9216/1 -- To view, visit http://gerrit.cloudera.org:8080/9216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I084e0944f388c22e838b017663a812b0ba77245d Gerrit-Change-Number: 9216 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9199 to look at the new patch set (#6). Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. KUDU-2281: Add Primary Key Support For INT128 Columns Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd --- M src/kudu/client/predicate-test.cc M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/common/encoded_key-test.cc M src/kudu/common/key_encoder.cc M src/kudu/common/key_encoder.h M src/kudu/common/key_util-test.cc M src/kudu/common/key_util.cc M src/kudu/common/partial_row.h M src/kudu/integration-tests/all_types-itest.cc 10 files changed, 128 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/9199/6 -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 6 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9199 ) Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 5 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 18:58:52 + Gerrit-HasComments: No
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9199 ) Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h File src/kudu/common/key_encoder.h: http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@94 PS3, Line 94: > It may be better to do static_cast(1) here, so that it d Done http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@116 PS3, Line 116: } > likewise Done http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc File src/kudu/integration-tests/all_types-itest.cc: http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc@291 PS3, Line 291: builder.AddColumn("decimal32_val")->Type(KuduColumnSchema::DECIMAL) > Perhaps this should have a decimal column from each size bucket? Done -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 5 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 18:47:24 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-721: [Spark] Add DECIMAL type support
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9213 Change subject: KUDU-721: [Spark] Add DECIMAL type support .. KUDU-721: [Spark] Add DECIMAL type support Adds DECIMAL support to the Kudu Spark source and RDD. Change-Id: Ia5f7a801778ed81b68949bbf8d7c08d1a13ed840 --- M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala M java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduRDD.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduContextTest.scala M java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/TestContext.scala 6 files changed, 84 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/9213/1 -- To view, visit http://gerrit.cloudera.org:8080/9213 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ia5f7a801778ed81b68949bbf8d7c08d1a13ed840 Gerrit-Change-Number: 9213 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9199 to look at the new patch set (#4). Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. KUDU-2281: Add Primary Key Support For INT128 Columns Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd --- M src/kudu/client/predicate-test.cc M src/kudu/client/schema.cc M src/kudu/client/schema.h M src/kudu/common/encoded_key-test.cc M src/kudu/common/key_encoder.cc M src/kudu/common/key_encoder.h M src/kudu/common/key_util-test.cc M src/kudu/common/key_util.cc M src/kudu/common/partial_row.h M src/kudu/integration-tests/all_types-itest.cc 10 files changed, 125 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/9199/4 -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 4 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [docs] Added recommendation to compress PK for backfill inserts
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9185 ) Change subject: [docs] Added recommendation to compress PK for backfill inserts .. [docs] Added recommendation to compress PK for backfill inserts Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7 Reviewed-on: http://gerrit.cloudera.org:8080/9185 Reviewed-by: Dan BurkertTested-by: Dan Burkert --- M docs/schema_design.adoc 1 file changed, 64 insertions(+), 18 deletions(-) Approvals: Dan Burkert: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/9185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7 Gerrit-Change-Number: 9185 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Rodoni Gerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [docs] Added recommendation to compress PK for backfill inserts
Dan Burkert has removed a vote on this change. Change subject: [docs] Added recommendation to compress PK for backfill inserts .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/9185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7 Gerrit-Change-Number: 9185 Gerrit-PatchSet: 4 Gerrit-Owner: Alex RodoniGerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [docs] Added recommendation to compress PK for backfill inserts
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9185 ) Change subject: [docs] Added recommendation to compress PK for backfill inserts .. Patch Set 4: Verified+1 Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I698d954265b4171e4d1bb7e01e286d0d489f1ec7 Gerrit-Change-Number: 9185 Gerrit-PatchSet: 4 Gerrit-Owner: Alex RodoniGerrit-Reviewer: Alex Rodoni Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Comment-Date: Mon, 05 Feb 2018 18:27:22 + Gerrit-HasComments: No
[kudu-CR] [tests] Add more test coverage for INT128 columns
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9193 ) Change subject: [tests] Add more test coverage for INT128 columns .. [tests] Add more test coverage for INT128 columns Improves test coverage of reading and writing INT128 columns and fixes a predicate issue exposed by those tests. Also adds INT64 tests where it was missing. Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295 Reviewed-on: http://gerrit.cloudera.org:8080/9193 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert--- M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/encoding-test.cc M src/kudu/client/scan_batch.cc M src/kudu/common/column_predicate.cc M src/kudu/common/partial_row.cc M src/kudu/tablet/all_types-scan-correctness-test.cc 6 files changed, 69 insertions(+), 11 deletions(-) Approvals: Kudu Jenkins: Verified Dan Burkert: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295 Gerrit-Change-Number: 9193 Gerrit-PatchSet: 10 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [docs] Add scaling guide
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8842 to look at the new patch set (#3). Change subject: [docs] Add scaling guide .. [docs] Add scaling guide This adds some more detailed information on how Kudu scales w.r.t several resources and provides some background on the scale limits and how to plan capacity for a Kudu deployment. Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2 --- A docs/scaling_guide.adoc 1 file changed, 182 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/8842/3 -- To view, visit http://gerrit.cloudera.org:8080/8842 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2 Gerrit-Change-Number: 8842 Gerrit-PatchSet: 3 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [docs] Add scaling guide
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8842 ) Change subject: [docs] Add scaling guide .. Patch Set 2: New rendering at https://github.com/wdberkeley/kudu/blob/scalingdoc/docs/scaling_guide.adoc Note the "Warning" box shows up as a big exclamation mark when rendered on the Kudu website, e.g. http://kudu.apache.org/docs/administration.html#_collecting_metrics_to_a_log. -- To view, visit http://gerrit.cloudera.org:8080/8842 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2 Gerrit-Change-Number: 8842 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Feb 2018 18:08:55 + Gerrit-HasComments: No
[kudu-CR] [docs] Add scaling guide
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8842 ) Change subject: [docs] Add scaling guide .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/8842/2/docs/scaling_guide.adoc File docs/scaling_guide.adoc: http://gerrit.cloudera.org:8080/#/c/8842/2/docs/scaling_guide.adoc@22 PS2, Line 22: The recommendations and conclusions here are only approximations. Appropriate numbers depend on use : case. There is no substitute for measurement and monitoring of resources used during a : representative workload. > is there some adoc syntax we can use to make this show up with a big warnin Done http://gerrit.cloudera.org:8080/#/c/8842/2/docs/scaling_guide.adoc@88 PS2, Line 88: | Total | 38.5GB > maybe make two separate rows here -- one for "expected memory usage" and on Done -- To view, visit http://gerrit.cloudera.org:8080/8842 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I38d8999addc41fe0b726342a27dbba199ddf7dd2 Gerrit-Change-Number: 8842 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Feb 2018 18:07:51 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Document how to recover from a majority failed tablet
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/8402 ) Change subject: [docs] Document how to recover from a majority failed tablet .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/8402/6/docs/administration.adoc File docs/administration.adoc: http://gerrit.cloudera.org:8080/#/c/8402/6/docs/administration.adoc@841 PS6, Line 841: .I > tiny nit: missing space Done -- To view, visit http://gerrit.cloudera.org:8080/8402 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6326f65d029a1cd75e487b16ce5be4baea2f215 Gerrit-Change-Number: 8402 Gerrit-PatchSet: 6 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Feb 2018 17:58:39 + Gerrit-HasComments: Yes
[kudu-CR] [docs] Document how to recover from a majority failed tablet
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8402 to look at the new patch set (#7). Change subject: [docs] Document how to recover from a majority failed tablet .. [docs] Document how to recover from a majority failed tablet This adds some docs on how to recover when a tablet can no longer find a majority due to the permanent failure of replicas. I tested this procedure by failing tablets in various ways: - deleting important bits like cmeta or tablet metadata - deleting entire data dirs - tombstoning 2/3 replicas (and disabling tombstoned voting) and I was always able to recover using these instructions. Change-Id: Ic6326f65d029a1cd75e487b16ce5be4baea2f215 --- M docs/administration.adoc 1 file changed, 65 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/8402/7 -- To view, visit http://gerrit.cloudera.org:8080/8402 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic6326f65d029a1cd75e487b16ce5be4baea2f215 Gerrit-Change-Number: 8402 Gerrit-PatchSet: 7 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] [tests] Add more test coverage for INT128 columns
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9193 ) Change subject: [tests] Add more test coverage for INT128 columns .. Patch Set 9: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295 Gerrit-Change-Number: 9193 Gerrit-PatchSet: 9 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 17:50:14 + Gerrit-HasComments: No
[kudu-CR] KUDU-2281: Add Primary Key Support For INT128 Columns
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9199 ) Change subject: KUDU-2281: Add Primary Key Support For INT128 Columns .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/encoded_key-test.cc File src/kudu/common/encoded_key-test.cc: http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/encoded_key-test.cc@178 PS3, Line 178: EXPECT_DECODED_KEY_EQ(INT128, "(int128 key=170141183460469231731687303715884105727)", Add a negative value test case. http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h File src/kudu/common/key_encoder.h: http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@94 PS3, Line 94: uint128_t It may be better to do static_cast(1) here, so that it doesn't use 128bit intermediate types for int8, int32, etc. Not sure if that would cause a performance difference, but seems simpler to just avoid it entirely anyways. http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/common/key_encoder.h@116 PS3, Line 116: val ^= static_cast(1) << (sizeof(val) * CHAR_BIT - 1); likewise http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc File src/kudu/integration-tests/all_types-itest.cc: http://gerrit.cloudera.org:8080/#/c/9199/3/src/kudu/integration-tests/all_types-itest.cc@291 PS3, Line 291: builder.AddColumn("decimal_val")->Type(KuduColumnSchema::DECIMAL) Perhaps this should have a decimal column from each size bucket? -- To view, visit http://gerrit.cloudera.org:8080/9199 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic6aadca2c5207f397ef6bdde4462a7497fea8ccd Gerrit-Change-Number: 9199 Gerrit-PatchSet: 3 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 16:26:28 + Gerrit-HasComments: Yes
[kudu-CR] [tests] Add more test coverage for INT128 columns
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/9193 ) Change subject: [tests] Add more test coverage for INT128 columns .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc File src/kudu/cfile/cfile-test.cc: http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc@563 PS8, Line 563: // Test for BitShuffle builder for UINT8, INT8, UINT16, INT16, UINT32, INT32, > Update this comment. Also, is the omission of the 64 bit types intentional Done http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc File src/kudu/cfile/encoding-test.cc: http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc@694 PS8, Line 694: // Test for bitshuffle block, for INT32, INT64, INT128, FLOAT, DOUBLE > Same here, seems like 64bit should be added as well. Done -- To view, visit http://gerrit.cloudera.org:8080/9193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295 Gerrit-Change-Number: 9193 Gerrit-PatchSet: 9 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 16:20:45 + Gerrit-HasComments: Yes
[kudu-CR] [tests] Add more test coverage for INT128 columns
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9193 to look at the new patch set (#9). Change subject: [tests] Add more test coverage for INT128 columns .. [tests] Add more test coverage for INT128 columns Improves test coverage of reading and writing INT128 columns and fixes a predicate issue exposed by those tests. Also adds INT64 tests where it was missing. Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295 --- M src/kudu/cfile/cfile-test.cc M src/kudu/cfile/encoding-test.cc M src/kudu/client/scan_batch.cc M src/kudu/common/column_predicate.cc M src/kudu/common/partial_row.cc M src/kudu/tablet/all_types-scan-correctness-test.cc 6 files changed, 69 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/9193/9 -- To view, visit http://gerrit.cloudera.org:8080/9193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295 Gerrit-Change-Number: 9193 Gerrit-PatchSet: 9 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot
[kudu-CR] [tests] Add more test coverage for INT128 columns
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/9193 ) Change subject: [tests] Add more test coverage for INT128 columns .. Patch Set 8: (2 comments) LGTM, but seems like it may be a good opportunity to fill in the 64-bit tests as well. http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc File src/kudu/cfile/cfile-test.cc: http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/cfile-test.cc@563 PS8, Line 563: // Test for BitShuffle builder for UINT8, INT8, UINT16, INT16, UINT32, INT32, FLOAT, DOUBLE Update this comment. Also, is the omission of the 64 bit types intentional here? Seems like maybe we overlooked it at some point. http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc File src/kudu/cfile/encoding-test.cc: http://gerrit.cloudera.org:8080/#/c/9193/8/src/kudu/cfile/encoding-test.cc@694 PS8, Line 694: // Test for bitshuffle block, for INT32, INT128, FLOAT, DOUBLE Same here, seems like 64bit should be added as well. -- To view, visit http://gerrit.cloudera.org:8080/9193 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3350634e55aad4316036c80d03e95e0acdf64295 Gerrit-Change-Number: 9193 Gerrit-PatchSet: 8 Gerrit-Owner: Grant HenkeGerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Comment-Date: Mon, 05 Feb 2018 16:08:24 + Gerrit-HasComments: Yes
[kudu-CR] Improved logging of DMS flushes
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9204 ) Change subject: Improved logging of DMS flushes .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc File src/kudu/tablet/delta_tracker.cc: http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc@669 PS1, Line 669: bytes > can you clarify this as "on disk" Done http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc@680 PS1, Line 680: LOG_WITH_PREFIX(INFO) << "Opened new delta block " << block_id.ToString() << " for read"; > I wonder if this log is really useful - seems somewhat redundant with the a Done http://gerrit.cloudera.org:8080/#/c/9204/1/src/kudu/tablet/delta_tracker.cc@727 PS1, Line 727: bytes > can you clarify "in memory" here to contrast from the bytes that we report Done -- To view, visit http://gerrit.cloudera.org:8080/9204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65 Gerrit-Change-Number: 9204 Gerrit-PatchSet: 1 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Mon, 05 Feb 2018 08:47:26 + Gerrit-HasComments: Yes
[kudu-CR] Improved logging of DMS flushes
Hello Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9204 to look at the new patch set (#2). Change subject: Improved logging of DMS flushes .. Improved logging of DMS flushes >From compaction-test.cc's TestRowSetInput, Before: I0202 14:52:24.983525 2427953984 delta_tracker.cc:727] T test_tablet_id P a3e9bdf25f0b431ea57d0249535c29b3: Flushing 20 deltas from DMS 0... I0202 14:52:24.984046 2427953984 delta_tracker.cc:668] T test_tablet_id P a3e9bdf25f0b431ea57d0249535c29b3: Flushed delta block: 0198399763226209 ts range: [11, 30] After: I0202 14:45:18.924826 2427953984 delta_tracker.cc:726] T test_tablet_id P 37440651650a4d858267ad408b7e1ac0: Flushing 20 deltas (1759 bytes on disk) from DMS 0 I0202 14:45:18.925750 2427953984 delta_tracker.cc:668] T test_tablet_id P 37440651650a4d858267ad408b7e1ac0: Flushed delta block 0052242700095437 (296 bytes in memory) stats: ts range=[11, 30], delete_count=[0], reinsert_count=[0], update_counts_by_col_id=[12:20,11:20] Plus one tiny change to clarify logging on flushes. Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65 --- M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/tablet.cc 3 files changed, 11 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/9204/2 -- To view, visit http://gerrit.cloudera.org:8080/9204 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8241a6ae33c22838504aa7007463d11167a22e65 Gerrit-Change-Number: 9204 Gerrit-PatchSet: 2 Gerrit-Owner: Will BerkeleyGerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon