[kudu-CR] Consider the available space when selecting data dirs for blocks.
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13975 )
Change subject: Consider the available space when selecting data dirs for
blocks.
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13975/6/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/13975/6/src/kudu/fs/data_dirs.cc@991
PS6, Line 991:
: if (FLAGS_refresh_is_full_with_expired_only_for_testing) {
: // This currently should only be reached by
disk_failure-itest.
: refresh_mode = DataDir::RefreshMode::EXPIRED_ONLY;
: }
> I prefer to change the EXPIRED_ONLY behaviors in RefreshIsFull such as igno
Yeah Andrew's proposal makes sense to me. We could probably also reduce the
caching time from 30s to something like 5s if you're worried about holding onto
a stale disk space value for too long. BTW, you might find
https://github.com/apache/kudu/commit/2a802f9f376de5175170a933bc8c35154f6eda92
interesting.
FWIW, I don't find the test-only gflag terribly offensive if the alternatives
are worse. Just make sure you tag it as HIDDEN.
--
To view, visit http://gerrit.cloudera.org:8080/13975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I194c4965ee64aed728e3b84e684c04d445cbe529
Gerrit-Change-Number: 13975
Gerrit-PatchSet: 6
Gerrit-Owner: ZhangYao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: ZhangYao
Gerrit-Comment-Date: Wed, 07 Aug 2019 04:23:46 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13426 ) Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. Patch Set 29: (1 comment) http://gerrit.cloudera.org:8080/#/c/13426/20/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/13426/20/src/kudu/master/catalog_manager.cc@4171 PS20, Line 4171: ts_desc->ToString(), tablet->ToString(), table_schema_version, > I don't think fluctuation of a metric value is particularly harmful either. I suspect a reasonable first step will be (once we expose e.g. live row count via RPC) to strongly document that in certain situations the value may not be accurate. Then if that causes problems, we can work towards hiding fluctuations on e.g. master restarts. -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 29 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 07 Aug 2019 03:53:01 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Hello Mike Percy, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13426 to look at the new patch set (#29). Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. KUDU-2797 p2: the master aggregates tablet statistics There are some jiras are talking about metrics: KUDU-1067, KUDU-1373, KUDU-2019, KUDU-2797. In this patch, it makes the following improvements: 1) disk size and live row count of the replicas that are the leadership roles are aggregated on the master server; 2) disk size and live row count of the table are exposed as metrics on the master server; 3) disk size and live row count of the table are exposed on the master server's Web-UI; Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc A src/kudu/master/table_metrics.cc A src/kudu/master/table_metrics.h M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/heartbeater.h M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/util/metrics.cc M www/table.mustache 22 files changed, 751 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/13426/29 -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 29 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
helifu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13426 ) Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. Patch Set 28: (3 comments) http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@735 PS25, Line 735: _master->Shutdown(); > Right, I meant changing these values to lower values so that the test doesn Done http://gerrit.cloudera.org:8080/#/c/13426/27/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/13426/27/src/kudu/integration-tests/ts_tablet_manager-itest.cc@732 PS27, Line 732: int idx = 0; > From the test failure, it seems like this isn't getting updated for some re Done http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/tablet/tablet_replica.cc@831 PS25, Line 831: : // The following line of code should precede "lock_", : // otherwise a deadlock will result. > I see. There's a comment in TabletReplica::Start() that explains this a bit Done -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 28 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 07 Aug 2019 03:01:59 + Gerrit-HasComments: Yes
[kudu-CR] Consider the available space when selecting data dirs for blocks.
ZhangYao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13975 )
Change subject: Consider the available space when selecting data dirs for
blocks.
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13975/6/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/13975/6/src/kudu/fs/data_dirs.cc@991
PS6, Line 991:
: if (FLAGS_refresh_is_full_with_expired_only_for_testing) {
: // This currently should only be reached by
disk_failure-itest.
: refresh_mode = DataDir::RefreshMode::EXPIRED_ONLY;
: }
> I see. What if we took a slightly different approach and:
I prefer to change the EXPIRED_ONLY behaviors in RefreshIsFull such as ignore
the is_full_ condition and focus on expired time. I will ask adr for advices :)
--
To view, visit http://gerrit.cloudera.org:8080/13975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I194c4965ee64aed728e3b84e684c04d445cbe529
Gerrit-Change-Number: 13975
Gerrit-PatchSet: 6
Gerrit-Owner: ZhangYao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: ZhangYao
Gerrit-Comment-Date: Wed, 07 Aug 2019 02:32:32 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Hello Mike Percy, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13426 to look at the new patch set (#28). Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. KUDU-2797 p2: the master aggregates tablet statistics There are some jiras are talking about metrics: KUDU-1067, KUDU-1373, KUDU-2019, KUDU-2797. In this patch, it makes the following improvements: 1) disk size and live row count of the replicas that are the leadership roles are aggregated on the master server; 2) disk size and live row count of the table are exposed as metrics on the master server; 3) disk size and live row count of the table are exposed on the master server's Web-UI; Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc A src/kudu/master/table_metrics.cc A src/kudu/master/table_metrics.h M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/heartbeater.h M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/util/metrics.cc M www/table.mustache 22 files changed, 750 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/13426/28 -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 28 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13426 ) Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. Patch Set 27: (3 comments) http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@735 PS25, Line 735: > I just want to check the stats after leader-follower switch. So, it's neces Right, I meant changing these values to lower values so that the test doesn't take as long to run, since these Sleep() statements account for 1.5s each of test runtime. It's not that important, since it's only a few seconds total, but it could we could set the heartbeat interval to 100ms or something. http://gerrit.cloudera.org:8080/#/c/13426/27/src/kudu/integration-tests/ts_tablet_manager-itest.cc File src/kudu/integration-tests/ts_tablet_manager-itest.cc: http://gerrit.cloudera.org:8080/#/c/13426/27/src/kudu/integration-tests/ts_tablet_manager-itest.cc@732 PS27, Line 732: NO_FATALS(CheckStats(kRowsCount)); >From the test failure, it seems like this isn't getting updated for some >reason. I looked into it a bit, but haven't gotten the root cause. It seems >like all the tablet servers reported to the master, but the live row count >ended up being 0. The test failure was in TSAN mode, so it might be that the failure only triggers under slower environments. Running this test with --stress_cpu_threads= might help in debugging. http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/tablet/tablet_replica.cc File src/kudu/tablet/tablet_replica.cc: http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/tablet/tablet_replica.cc@831 PS25, Line 831: : // The following line of code should precede "lock_", : // otherwise a deadlock will result. > Line1402 in raft_consensus.cc and Line606 in tablet_replica.cc I see. There's a comment in TabletReplica::Start() that explains this a bit too. Could you update this to explain a bit more? Something like: We cannot hold 'lock_' while calling RaftConsensus::role() because it may invoke TabletReplica::StartFollowerTransaction() and lead to a deadlock. -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 27 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu Gerrit-Comment-Date: Wed, 07 Aug 2019 02:03:57 + Gerrit-HasComments: Yes
[kudu-CR] [code style] Keep same code style in test files
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14016 ) Change subject: [code style] Keep same code style in test files .. [code style] Keep same code style in test files Change all ASSERT_NO_FATAL_FAILURE to NO_FATALS to keep the same code style in test files. In addition, delete out-of-date logic in client-test. Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Reviewed-on: http://gerrit.cloudera.org:8080/14016 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/cfile/bloomfile-test.cc M src/kudu/cfile/mt-bloomfile-test.cc M src/kudu/client/client-test.cc M src/kudu/common/wire_protocol-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/mt-log-test.cc M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/master/master-test.cc M src/kudu/rpc/rpc-test.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/composite-pushdown-test.cc M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/mt-rowset_delta_compaction-test.cc M src/kudu/tablet/tablet_random_access-test.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/util/env-test.cc M src/kudu/util/trace-test.cc 26 files changed, 261 insertions(+), 287 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 5 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: honeyhexin
[kudu-CR] Consider the available space when selecting data dirs for blocks.
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13975 )
Change subject: Consider the available space when selecting data dirs for
blocks.
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13975/6/src/kudu/fs/data_dirs.cc
File src/kudu/fs/data_dirs.cc:
http://gerrit.cloudera.org:8080/#/c/13975/6/src/kudu/fs/data_dirs.cc@991
PS6, Line 991:
: if (FLAGS_refresh_is_full_with_expired_only_for_testing) {
: // This currently should only be reached by
disk_failure-itest.
: refresh_mode = DataDir::RefreshMode::EXPIRED_ONLY;
: }
> 1. FLAGS_refresh_is_full_with_expired_only_for_testing
I see. What if we took a slightly different approach and:
- continue to _always_ use EXPIRED_ONLY here. That way, we hit the disk error
when we expect to in the test, i.e. we won't check for available space when we
create blocks, but we _will_ hit a disk error when we write to those blocks.
- update the EXPIRED_ONLY behavior to not take into account fullness, i.e.
remove the is_full_ check for EXPIRED_ONLY. That way, we're not constantly
running this space check, which might be expensive, especially if run for every
block creation. AFAICT the RefreshIsFull() was originally written with fullness
only in mind. Since we're trying to use it for more than just fullness,
updating it to be "RefreshAvailableSpace()" or something might be worth doing.
BTW another way to avoid this specific check for disk_failure-itest would be to
have disk_failure-itest fail the glob "/data/*data" so it matches only to
*.data and *.metadata files, rather than the entire directory. That said, I
would prefer we consider the proposal above.
Also curious if Adar agrees here.
--
To view, visit http://gerrit.cloudera.org:8080/13975
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I194c4965ee64aed728e3b84e684c04d445cbe529
Gerrit-Change-Number: 13975
Gerrit-PatchSet: 6
Gerrit-Owner: ZhangYao
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Reviewer: ZhangYao
Gerrit-Comment-Date: Wed, 07 Aug 2019 00:43:31 +
Gerrit-HasComments: Yes
[kudu-CR] [code style] Keep same code style in test files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14016 ) Change subject: [code style] Keep same code style in test files .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 4 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Wed, 07 Aug 2019 00:08:18 + Gerrit-HasComments: No
[kudu-CR] [code style] Keep same code style in test files
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14016 to look at the new patch set (#4). Change subject: [code style] Keep same code style in test files .. [code style] Keep same code style in test files Change all ASSERT_NO_FATAL_FAILURE to NO_FATALS to keep the same code style in test files. In addition, delete out-of-date logic in client-test. Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 --- M src/kudu/cfile/bloomfile-test.cc M src/kudu/cfile/mt-bloomfile-test.cc M src/kudu/client/client-test.cc M src/kudu/common/wire_protocol-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/mt-log-test.cc M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/master/master-test.cc M src/kudu/rpc/rpc-test.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/composite-pushdown-test.cc M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/mt-rowset_delta_compaction-test.cc M src/kudu/tablet/tablet_random_access-test.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/util/env-test.cc M src/kudu/util/trace-test.cc 26 files changed, 261 insertions(+), 287 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/14016/4 -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 4 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: honeyhexin
[kudu-CR] [code style] Keep same code style in test files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14016 ) Change subject: [code style] Keep same code style in test files .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 3 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Tue, 06 Aug 2019 23:46:45 + Gerrit-HasComments: No
[kudu-CR] [code style] Keep same code style in test files
honeyhexin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14016 ) Change subject: [code style] Keep same code style in test files .. Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/client/client-test.cc@a4313 PS2, Line 4313: : : : > Why remove the comment? As we have discussed offline, delete these useless comments and out-of-date logic. http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/client/client-test.cc@4304 PS2, Line 4304: > Reindent. Done http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/consensus_queue-test.cc@593 PS2, Line 593: NO_FATALS(UpdatePeerWatermarkToOp(, > Reindent the continuation lines here. Done http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/raft_consensus_quorum-test.cc File src/kudu/consensus/raft_consensus_quorum-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/raft_consensus_quorum-test.cc@1048 PS2, Line 1048: NO_FATALS(AssertDurableTermAndVote(kPeerIndex, last_op_id.term() + 1, > Reindent all of the continuation lines affected by your change in this file Done http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/util/env-test.cc@216 PS2, Line 216: NO_FATALS(ReadAndVerifyTestData(raf.get(), num_slices * slice_size * i, > Reindent. Done http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/util/env-test.cc@229 PS2, Line 229: NO_FATALS(ReadAndVerifyTestData(raf.get(), num_slices * slice_size * i, > Reindent. Done -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 3 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: honeyhexin Gerrit-Comment-Date: Tue, 06 Aug 2019 23:28:30 + Gerrit-HasComments: Yes
[kudu-CR] [code style] Keep same code style in test files
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14016 to look at the new patch set (#3). Change subject: [code style] Keep same code style in test files .. [code style] Keep same code style in test files Change all ASSERT_NO_FATAL_FAILURE to NO_FATALS to keep the same code style in test files. In addition, delete out-of-date logic in client-test. Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 --- M src/kudu/cfile/bloomfile-test.cc M src/kudu/cfile/mt-bloomfile-test.cc M src/kudu/client/client-test.cc M src/kudu/common/wire_protocol-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/mt-log-test.cc M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/master/master-test.cc M src/kudu/rpc/rpc-test.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/composite-pushdown-test.cc M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/mt-rowset_delta_compaction-test.cc M src/kudu/tablet/tablet_random_access-test.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/util/env-test.cc M src/kudu/util/trace-test.cc 26 files changed, 242 insertions(+), 255 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/14016/3 -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 3 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2849 Docker image for python kudu client
Sandish Kumar HN has posted comments on this change. ( http://gerrit.cloudera.org:8080/14000 ) Change subject: KUDU-2849 Docker image for python kudu client .. Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/14000/5/docker/Dockerfile File docker/Dockerfile: http://gerrit.cloudera.org:8080/#/c/14000/5/docker/Dockerfile@226 PS5, Line 226: ENV LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib64 > Is this required? it complains lie "You are using pip version 18.1, however version 19.2.1 is available.", but passes the step http://gerrit.cloudera.org:8080/#/c/14000/5/docker/Dockerfile@228 PS5, Line 228: # Copy the requirements file. > Move this with the other COPY statements. You can use the same workdir as b done http://gerrit.cloudera.org:8080/#/c/14000/5/docker/Dockerfile@231 PS5, Line 231: COPY --from=build /kudu/python/dist/kudu-python-*.tar.gz . > I think this can all be done in one step/layer in the run below. We should done -- To view, visit http://gerrit.cloudera.org:8080/14000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7cab8ffe114c73752c261f27ebc7a58e4b57a6e Gerrit-Change-Number: 14000 Gerrit-PatchSet: 6 Gerrit-Owner: Sandish Kumar HN Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Sandish Kumar HN Gerrit-Comment-Date: Tue, 06 Aug 2019 22:21:24 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2849 Docker image for python kudu client
Hello Kudu Jenkins, Grant Henke, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14000 to look at the new patch set (#6). Change subject: KUDU-2849 Docker image for python kudu client .. KUDU-2849 Docker image for python kudu client A base image that has all the development libraries required to run Python Kudu Client. Building Python Kudu Client Image: Set Env export TARGETS=kudu-python ./docker/docker-build.sh Run docker kudu python client: docker run --name kudu-python -it apache/kudu:kudu-python-latest Change-Id: Ia7cab8ffe114c73752c261f27ebc7a58e4b57a6e --- M docker/Dockerfile M docker/README.adoc 2 files changed, 40 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/14000/6 -- To view, visit http://gerrit.cloudera.org:8080/14000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia7cab8ffe114c73752c261f27ebc7a58e4b57a6e Gerrit-Change-Number: 14000 Gerrit-PatchSet: 6 Gerrit-Owner: Sandish Kumar HN Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Sandish Kumar HN
[kudu-CR] POC: Disable Sentry related tests
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14020 ) Change subject: POC: Disable Sentry related tests .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/14020/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14020/1//COMMIT_MSG@7 PS1, Line 7: POC: Disable Sentry related tests May also want to disable the mini cluster options that allow Sentry to be configured. And perhaps an equivalent change in tool_action_test.cc. http://gerrit.cloudera.org:8080/#/c/14020/1/src/kudu/hms/mini_hms.cc File src/kudu/hms/mini_hms.cc: http://gerrit.cloudera.org:8080/#/c/14020/1/src/kudu/hms/mini_hms.cc@127 PS1, Line 127: reoved removed http://gerrit.cloudera.org:8080/#/c/14020/1/src/kudu/integration-tests/CMakeLists.txt File src/kudu/integration-tests/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/14020/1/src/kudu/integration-tests/CMakeLists.txt@93 PS1, Line 93: # NOTE: Sentry tests are disabled to allow upgrading to Hive 3. : #ADD_KUDU_TEST(master_sentry-itest RUN_SERIAL true NUM_SHARDS 8 PROCESSORS 4) This means we won't even compile the binary though. Could we disable these tests some other way? -- To view, visit http://gerrit.cloudera.org:8080/14020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7b99b0de221f6d4acfb427f830cd3344c8d0a10b Gerrit-Change-Number: 14020 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 06 Aug 2019 17:54:22 + Gerrit-HasComments: Yes
[kudu-CR] Prepare for upgrading to Hive 3
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/14018 )
Change subject: Prepare for upgrading to Hive 3
..
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/14018/1//COMMIT_MSG
Commit Message:
PS1:
Just curious: why are you wrapping your commit messages so aggressively? The
longest line here is 52 characters; I think 70ish (I use 76) is more typical.
http://gerrit.cloudera.org:8080/#/c/14018/1//COMMIT_MSG@13
PS1, Line 13: initialized
initialize
http://gerrit.cloudera.org:8080/#/c/14018/1//COMMIT_MSG@17
PS1, Line 17: MiniHMS
Nit: MiniHms (or change MiniHms above to MiniHMS).
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/hms_catalog.cc
File src/kudu/hms/hms_catalog.cc:
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/hms_catalog.cc@233
PS1, Line 233: // NOTE: LIKE filters are used instead of = filters due to
HIVE-21614
Does this have a tangible impact on performance (on a non-Derby HMS)? Any way
to measure?
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc@158
PS1, Line 158:
: // Set HADOOP_OS_TYPE=Linux due to HADOOP-8719.
: // Remove after HADOOP-15966 is available (Hadoop 3.1.3+)
: env_vars.insert({ "HADOOP_OS_TYPE", "Linux" });
Can this be combined into the env_vars declaration above?
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc@310
PS1, Line 310:
Please rationalize these new parameters above.
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/hms/mini_hms.cc@320
PS1, Line 320:
:
: hive.async.log.enabled
: false
:
I experimented with this but didn't find that it made any difference w.r.t.
logging. Is it actually important? Or can it be removed?
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/util/subprocess.cc
File src/kudu/util/subprocess.cc:
http://gerrit.cloudera.org:8080/#/c/14018/1/src/kudu/util/subprocess.cc@726
PS1, Line 726: p.SetEnvVars(env_vars);
Could you pass env_vars by value and std::move() it here?
--
To view, visit http://gerrit.cloudera.org:8080/14018
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If43ae2330b3d99374c68bae313a3f8bc070f9c69
Gerrit-Change-Number: 14018
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 06 Aug 2019 17:51:32 +
Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/13937 )
Change subject: [ksck] Filter tables and tablets in KsckCluster
..
Patch Set 9:
(4 comments)
The TSAN test failure looks real: a data race in the new code in KsckCluster.
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h
File src/kudu/tools/ksck.h:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h@471
PS9, Line 471: const int32_t& filtered_tables_count() const {
: return filtered_tables_count_;
: }
:
: const int32_t& filtered_tablets_count() const {
: return filtered_tablets_count_;
: }
Value types (i.e. int, double, etc.) should be returned by value rather than by
cref.
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck.h@489
PS9, Line 489: int32_t filtered_tables_count_;
: int32_t filtered_tablets_count_;
Nit: 'int' is fine too.
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_checksum.cc
File src/kudu/tools/ksck_checksum.cc:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_checksum.cc@491
PS9, Line 491: &
&& n
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_remote.cc
File src/kudu/tools/ksck_remote.cc:
http://gerrit.cloudera.org:8080/#/c/13937/9/src/kudu/tools/ksck_remote.cc@526
PS9, Line 526: filtered_tables_count_ = 0;
Should probably also initialize these new variables to 0 in the constructor so
that they aren't garbage before RetrieveTablesList or RetrieveTabletsList are
called.
--
To view, visit http://gerrit.cloudera.org:8080/13937
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761
Gerrit-Change-Number: 13937
Gerrit-PatchSet: 9
Gerrit-Owner: Yifan Zhang
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang
Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 17:37:50 +
Gerrit-HasComments: Yes
[kudu-CR] [code style] Keep same code style in test files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14016 ) Change subject: [code style] Keep same code style in test files .. Patch Set 2: (6 comments) http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/client/client-test.cc@a4313 PS2, Line 4313: : : : Why remove the comment? http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/client/client-test.cc@4304 PS2, Line 4304: NO_FATALS(InsertTestRows(client_.get(), Reindent. http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/consensus_queue-test.cc File src/kudu/consensus/consensus_queue-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/consensus_queue-test.cc@593 PS2, Line 593: NO_FATALS(UpdatePeerWatermarkToOp(, Reindent the continuation lines here. http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/raft_consensus_quorum-test.cc File src/kudu/consensus/raft_consensus_quorum-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/consensus/raft_consensus_quorum-test.cc@1048 PS2, Line 1048: NO_FATALS(AssertDurableTermAndVote(kPeerIndex, last_op_id.term() + 1, Reindent all of the continuation lines affected by your change in this file. http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/util/env-test.cc File src/kudu/util/env-test.cc: http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/util/env-test.cc@216 PS2, Line 216: NO_FATALS(ReadAndVerifyTestData(raf.get(), num_slices * slice_size * i, Reindent. http://gerrit.cloudera.org:8080/#/c/14016/2/src/kudu/util/env-test.cc@229 PS2, Line 229: NO_FATALS(ReadAndVerifyTestData(raf.get(), num_slices * slice_size * i, Reindent. -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 2 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 06 Aug 2019 17:31:44 + Gerrit-HasComments: Yes
[kudu-CR] POC: Upgrade Hive dependency to 3.1.1
Hello Tidy Bot, Kudu Jenkins, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13256 to look at the new patch set (#12). Change subject: POC: Upgrade Hive dependency to 3.1.1 .. POC: Upgrade Hive dependency to 3.1.1 This patch - Upgrades Hive to 3.1.1 - Upgrades Hadoop to 3.2.0 Change-Id: Iec3ab2cc4e41a26cee8dd7f7098a2f288c56a42c --- M java/gradle/dependencies.gradle M java/kudu-hive/build.gradle M java/kudu-hive/src/test/java/org/apache/kudu/hive/metastore/TestKuduMetastorePlugin.java M src/kudu/hms/hive_metastore.thrift M thirdparty/vars.sh 5 files changed, 808 insertions(+), 97 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/13256/12 -- To view, visit http://gerrit.cloudera.org:8080/13256 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iec3ab2cc4e41a26cee8dd7f7098a2f288c56a42c Gerrit-Change-Number: 13256 Gerrit-PatchSet: 12 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] POC: Disable Sentry related tests
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14020 Change subject: POC: Disable Sentry related tests .. POC: Disable Sentry related tests This is done as an experiment to enable upgrading to Hive 3. Change-Id: I7b99b0de221f6d4acfb427f830cd3344c8d0a10b --- M src/kudu/hms/mini_hms.cc M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/alter_table-randomized-test.cc M src/kudu/integration-tests/master-stress-test.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/sentry/CMakeLists.txt 7 files changed, 28 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/14020/1 -- To view, visit http://gerrit.cloudera.org:8080/14020 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7b99b0de221f6d4acfb427f830cd3344c8d0a10b Gerrit-Change-Number: 14020 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Hello Mike Percy, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13426 to look at the new patch set (#27). Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. KUDU-2797 p2: the master aggregates tablet statistics There are some jiras are talking about metrics: KUDU-1067, KUDU-1373, KUDU-2019, KUDU-2797. In this patch, it makes the following improvements: 1) disk size and live row count of the replicas that are the leadership roles are aggregated on the master server; 2) disk size and live row count of the table are exposed as metrics on the master server; 3) disk size and live row count of the table are exposed on the master server's Web-UI; Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc A src/kudu/master/table_metrics.cc A src/kudu/master/table_metrics.h M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/heartbeater.h M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/util/metrics.cc M www/table.mustache 22 files changed, 740 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/13426/27 -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 27 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
Hello Mike Percy, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13426 to look at the new patch set (#26). Change subject: KUDU-2797 p2: the master aggregates tablet statistics .. KUDU-2797 p2: the master aggregates tablet statistics There are some jiras are talking about metrics: KUDU-1067, KUDU-1373, KUDU-2019, KUDU-2797. In this patch, it makes the following improvements: 1) disk size and live row count of the replicas that are the leadership roles are aggregated on the master server; 2) disk size and live row count of the table are exposed as metrics on the master server; 3) disk size and live row count of the table are exposed on the master server's Web-UI; Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 --- M src/kudu/integration-tests/cluster_itest_util.cc M src/kudu/integration-tests/cluster_itest_util.h M src/kudu/integration-tests/tablet_replacement-itest.cc M src/kudu/integration-tests/ts_tablet_manager-itest.cc M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc A src/kudu/master/table_metrics.cc A src/kudu/master/table_metrics.h M src/kudu/tablet/local_tablet_writer.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/tablet_replica.cc M src/kudu/tablet/tablet_replica.h M src/kudu/tserver/heartbeater.cc M src/kudu/tserver/heartbeater.h M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h M src/kudu/util/metrics.cc M www/table.mustache 22 files changed, 742 insertions(+), 35 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/26/13426/26 -- To view, visit http://gerrit.cloudera.org:8080/13426 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I74406ab7cca7c22fda455c328b8ee9989a6b2d99 Gerrit-Change-Number: 13426 Gerrit-PatchSet: 26 Gerrit-Owner: helifu Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: helifu
[kudu-CR] Prepare for upgrading to Hive 3
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14018 Change subject: Prepare for upgrading to Hive 3 .. Prepare for upgrading to Hive 3 This patch contains build and test changes that work on both Hive 2 and Hive 3 to minimize the changes required when upgrading to Hive 3. - Uses the Hive `schematool` to initialized the derby database in the MiniHms. This fixes issues with autoCreate and is more representative of a production environment. - Adjust logging configuration in the MiniHMS to be more explicit. - Workaround HADOOP-8719 by hardcoding the HADOOP_OS_TYPE. - Workaround HIVE-21614 by using `LIKE` instead of `=` when filtering tables. Change-Id: If43ae2330b3d99374c68bae313a3f8bc070f9c69 --- M src/kudu/hms/CMakeLists.txt M src/kudu/hms/hms_catalog.cc M src/kudu/hms/hms_client-test.cc M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h M src/kudu/util/subprocess.cc M src/kudu/util/subprocess.h M thirdparty/package-hadoop.sh 8 files changed, 55 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/18/14018/1 -- To view, visit http://gerrit.cloudera.org:8080/14018 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: If43ae2330b3d99374c68bae313a3f8bc070f9c69 Gerrit-Change-Number: 14018 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2797 p2: the master aggregates tablet statistics
helifu has posted comments on this change. (
http://gerrit.cloudera.org:8080/13426 )
Change subject: KUDU-2797 p2: the master aggregates tablet statistics
..
Patch Set 25:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/13426/25//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13426/25//COMMIT_MSG@13
PS25, Line 13: all the replicas are
:aggregated
> This is no longer true, right?
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc
File src/kudu/integration-tests/ts_tablet_manager-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@603
PS25, Line 603: const auto RunUDF = [&] (const std::function&
udf_function) {
> nit: I think it would be easier to follow this code if these lambdas return
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@603
PS25, Line 603: RunUDF
> nit: could you name this something more descriptive, like GetLeaderMasterAn
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@618
PS25, Line 618: check_disk_size = disk_size;
: check_live_row_count = live_row_count;
> nit: why not just use disk_size and live_row_count directly?
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@720
PS25, Line 720: strMetricAttrs
> nit: metric_attrs_str for variable names.
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@724
PS25, Line 724: CheckFunction
> nit: you can simply pass this in as:
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@735
PS25, Line 735: FLAGS_raft_heartbeat_interval_ms *
FLAGS_leader_failure_max_missed_heartbeat_periods
> Since we're sleeping for these durations, how about setting these to be low
I just want to check the stats after leader-follower switch. So, it's necessary
to wait. ^_^
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@769
PS25, Line 769: strMetrics
> nit: metrics_str
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/integration-tests/ts_tablet_manager-itest.cc@770
PS25, Line 770: // Return the metric entity and set a retired time.
: NO_FATALS(GetMetricsString());
: ASSERT_STR_CONTAINS(strMetrics, kNewTableName);
: // Return the metric entity and retire it.
: NO_FATALS(GetMetricsString());
: ASSERT_STR_CONTAINS(strMetrics, kNewTableName);
: // The metric entity has been retired.
: NO_FATALS(GetMetricsString());
: ASSERT_STR_NOT_CONTAINS(strMetrics, kNewTableName);
> Hrm, I'm a bit confused about this set of assertions. Maybe I'm missing som
Yeah, the process is a little confusing.
It relies on the logic of the metrics(metrics.cc Line377). I have updated the
comments and hope it helps.
http://gerrit.cloudera.org:8080/#/c/13426/20/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/13426/20/src/kudu/master/catalog_manager.cc@4171
PS20, Line 4171: ts_desc->ToString(), tablet->ToString(),
table_schema_version,
> Thinking about this more, I don't think fluctuation of a metric value is pa
I don't think fluctuation of a metric value is particularly harmful either. And
at the same time, I suggest that these aggregated metrics could be used to
optimize the query plans. I know Andrew is worrying about the metrics'
freshness and Adar is worrying about metrics' fluctuation, but these metrics
only __affect the query plan, not the accuracy of the query results___.
In addition, in a production cluster, it's impossible to shutdown more than one
tserver at the same time. That means the fluctuations tserver causes are
nonexistent.But if it does, not only the metrics are inaccurate but the data
are lost potentially. And for the restarted master which is the leader, the
time window for all tservers to report metrics is very short.
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/master/master_path_handlers.cc
File src/kudu/master/master_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/master/master_path_handlers.cc@433
PS25, Line 433: (*output)["table_disk_size"] =
:
HumanReadableNumBytes::ToString(table_metrics->on_disk_size->value());
> If I understand correctly, the negative case is in case the master hasn't r
Done
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/master/table_metrics.cc
File src/kudu/master/table_metrics.cc:
http://gerrit.cloudera.org:8080/#/c/13426/25/src/kudu/master/table_metrics.cc@26
PS25, Line 26:
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/13937 ) Change subject: [ksck] Filter tables and tablets in KsckCluster .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc File src/kudu/tools/ksck_checksum.cc: http://gerrit.cloudera.org:8080/#/c/13937/5/src/kudu/tools/ksck_checksum.cc@502 PS5, Line 502: // The table may have no tablets if all range partitions have been dropped. > Why should this be an error? If there were tablets but the user filtered them > all out, shouldn't this be Status::OK? Maybe the error could tell users they set wrong filters. > Would it be difficult to add tracking of the list of filtered tables/tablets > in KsckCluster, and then use it to differentiate between the two cases here? Well, if we do this we actually track all tables and tablets, which was same as before. But the idea of this patch is to track less tables and tablets in KsckCluster to accelerate the execution of ksck tool. However, in ordered to differentiate between the two cases here, I added two variables named 'filtered_tables_count' and 'filtered_tablets_count' to KsckCluster. Besides I added some unit tests to coverage different cases, including the cluster has no table, the cluster has tables but all tables filtered out, the cluster has a table with no tablet, the cluster has a table with tablets but all tablets filtered out. -- To view, visit http://gerrit.cloudera.org:8080/13937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761 Gerrit-Change-Number: 13937 Gerrit-PatchSet: 9 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com> Gerrit-Comment-Date: Tue, 06 Aug 2019 11:48:00 + Gerrit-HasComments: Yes
[kudu-CR] [ksck] Filter tables and tablets in KsckCluster
Hello Alexey Serbin, Yingchun Lai, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13937 to look at the new patch set (#9). Change subject: [ksck] Filter tables and tablets in KsckCluster .. [ksck] Filter tables and tablets in KsckCluster The ksck tool executes slowly if there are too many tables in a cluster even if we just want to check some specific tables and tablets. Filtering tables and tablets in KsckCluster and only tracking specified tables and tablets would speed up the execution of ksck tool with '--tables=' and '--tablets=' args. Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761 --- M src/kudu/tools/ksck-test.cc M src/kudu/tools/ksck.cc M src/kudu/tools/ksck.h M src/kudu/tools/ksck_checksum.cc M src/kudu/tools/ksck_remote.cc M src/kudu/tools/rebalancer.cc M src/kudu/tools/tool_action_cluster.cc M src/kudu/tools/tool_replica_util.cc 8 files changed, 119 insertions(+), 39 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/37/13937/9 -- To view, visit http://gerrit.cloudera.org:8080/13937 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I23b6e6ef258d3498a42af7f92b63392a59c99761 Gerrit-Change-Number: 13937 Gerrit-PatchSet: 9 Gerrit-Owner: Yifan Zhang Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang Gerrit-Reviewer: Yingchun Lai <405403...@qq.com>
[kudu-CR] [code style] Keep same code style in test files
Hello Tidy Bot, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14016 to look at the new patch set (#2). Change subject: [code style] Keep same code style in test files .. [code style] Keep same code style in test files Change all ASSERT_NO_FATAL_FAILURE to NO_FATALS to keep the same code style in test files. Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 --- M src/kudu/cfile/bloomfile-test.cc M src/kudu/cfile/mt-bloomfile-test.cc M src/kudu/client/client-test.cc M src/kudu/common/wire_protocol-test.cc M src/kudu/consensus/consensus_queue-test.cc M src/kudu/consensus/leader_election-test.cc M src/kudu/consensus/mt-log-test.cc M src/kudu/consensus/raft_consensus_quorum-test.cc M src/kudu/fs/block_manager-test.cc M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/create-table-stress-test.cc M src/kudu/integration-tests/linked_list-test.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/integration-tests/update_scan_delta_compact-test.cc M src/kudu/master/master-test.cc M src/kudu/rpc/rpc-test.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/composite-pushdown-test.cc M src/kudu/tablet/major_delta_compaction-test.cc M src/kudu/tablet/mt-rowset_delta_compaction-test.cc M src/kudu/tablet/tablet_random_access-test.cc M src/kudu/tserver/tablet_server-test.cc M src/kudu/tserver/ts_tablet_manager-test.cc M src/kudu/util/env-test.cc M src/kudu/util/trace-test.cc 26 files changed, 228 insertions(+), 235 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/14016/2 -- To view, visit http://gerrit.cloudera.org:8080/14016 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I67689fe8113d88f60ac33fa38504813128209da3 Gerrit-Change-Number: 14016 Gerrit-PatchSet: 2 Gerrit-Owner: honeyhexin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
