You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/07/27 04:26:50 UTC

[kudu-CR] Move clock-related classes to src/kudu/clock

Todd Lipcon has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7519

Change subject: Move clock-related classes to src/kudu/clock
......................................................................

Move clock-related classes to src/kudu/clock

This creates a new module for clock-related classes (currently just
logical and hybrid clock). This is in preparation for introducing
a built-in NTP client.

Since these clock-related classes don't depend on anything else in the
server, it makes sense to move them into a new small module.

Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
---
M CMakeLists.txt
M src/kudu/client/client-test.cc
A src/kudu/clock/CMakeLists.txt
R src/kudu/clock/clock.h
R src/kudu/clock/hybrid_clock-test.cc
R src/kudu/clock/hybrid_clock.cc
R src/kudu/clock/hybrid_clock.h
R src/kudu/clock/logical_clock-test.cc
R src/kudu/clock/logical_clock.cc
R src/kudu/clock/logical_clock.h
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/authn_token_expire-itest.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/server/CMakeLists.txt
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
54 files changed, 91 insertions(+), 89 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/7519/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: Move clock-related classes to src/kudu/clock
......................................................................


Patch Set 3:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7519/4/src/kudu/clock/CMakeLists.txt
File src/kudu/clock/CMakeLists.txt:

PS4, Line 19:     hybrid_clock.cc
            :     logical_clock.cc)
Nit: indentation


http://gerrit.cloudera.org:8080/#/c/7519/4/src/kudu/clock/hybrid_clock.h
File src/kudu/clock/hybrid_clock.h:

Line 17: #pragma once
Mind updating the rest of clock/* to use this?


http://gerrit.cloudera.org:8080/#/c/7519/4/src/kudu/consensus/log-test-base.h
File src/kudu/consensus/log-test-base.h:

PS4, Line 40: #include "kudu/clock/clock.h"
            : #include "kudu/clock/hybrid_clock.h"
Nit: resort


Line 62: using clock::Clock;
Nit: resort


http://gerrit.cloudera.org:8080/#/c/7519/3/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:

Line 61: using kudu::clock::HybridClock;
Nit: resort


http://gerrit.cloudera.org:8080/#/c/7519/3/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

PS3, Line 62: using clock::Clock;
            : using clock::HybridClock;
Nit: resort


-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Move clock-related classes to src/kudu/clock
......................................................................


Patch Set 1:

(5 comments)

fixed all the include order issues and added missing cmake dependency from server -> codegen

http://gerrit.cloudera.org:8080/#/c/7519/1/src/kudu/consensus/time_manager-test.cc
File src/kudu/consensus/time_manager-test.cc:

Line 21: #include "kudu/consensus/consensus.pb.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


http://gerrit.cloudera.org:8080/#/c/7519/1/src/kudu/consensus/time_manager.h
File src/kudu/consensus/time_manager.h:

Line 24: #include "kudu/gutil/ref_counted.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


http://gerrit.cloudera.org:8080/#/c/7519/1/src/kudu/integration-tests/alter_table-test.cc
File src/kudu/integration-tests/alter_table-test.cc:

Line 42: #include "kudu/clock/hybrid_clock.h"
not sure why tidy-bot didn't complain about the include order here, but fixed...


http://gerrit.cloudera.org:8080/#/c/7519/1/src/kudu/server/generic_service.cc
File src/kudu/server/generic_service.cc:

Line 24: #include "kudu/gutil/map-util.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


http://gerrit.cloudera.org:8080/#/c/7519/1/src/kudu/tablet/tablet-harness.h
File src/kudu/tablet/tablet-harness.h:

Line 26: #include "kudu/consensus/log_anchor_registry.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7519

to look at the new patch set (#3).

Change subject: Move clock-related classes to src/kudu/clock
......................................................................

Move clock-related classes to src/kudu/clock

This creates a new module for clock-related classes (currently just
logical and hybrid clock). This is in preparation for introducing
a built-in NTP client.

Since these clock-related classes don't depend on anything else in the
server, it makes sense to move them into a new small module.

Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
---
M CMakeLists.txt
M src/kudu/client/client-test.cc
A src/kudu/clock/CMakeLists.txt
R src/kudu/clock/clock.h
R src/kudu/clock/hybrid_clock-test.cc
R src/kudu/clock/hybrid_clock.cc
R src/kudu/clock/hybrid_clock.h
R src/kudu/clock/logical_clock-test.cc
R src/kudu/clock/logical_clock.cc
R src/kudu/clock/logical_clock.h
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/authn_token_expire-itest.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/master/sys_catalog.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
63 files changed, 216 insertions(+), 215 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/7519/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: Move clock-related classes to src/kudu/clock
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7519/5/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:

PS5, Line 59: using clock::Clock;
            : using clock::LogicalClock;
PS4 had the right sort order.


-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7519

to look at the new patch set (#6).

Change subject: Move clock-related classes to src/kudu/clock
......................................................................

Move clock-related classes to src/kudu/clock

This creates a new module for clock-related classes (currently just
logical and hybrid clock). This is in preparation for introducing
a built-in NTP client.

Since these clock-related classes don't depend on anything else in the
server, it makes sense to move them into a new small module.

Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
---
M CMakeLists.txt
M src/kudu/client/client-test.cc
A src/kudu/clock/CMakeLists.txt
R src/kudu/clock/clock.h
R src/kudu/clock/hybrid_clock-test.cc
R src/kudu/clock/hybrid_clock.cc
R src/kudu/clock/hybrid_clock.h
R src/kudu/clock/logical_clock-test.cc
R src/kudu/clock/logical_clock.cc
R src/kudu/clock/logical_clock.h
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/authn_token_expire-itest.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/master/sys_catalog.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
63 files changed, 231 insertions(+), 242 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/7519/6
-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7519

to look at the new patch set (#5).

Change subject: Move clock-related classes to src/kudu/clock
......................................................................

Move clock-related classes to src/kudu/clock

This creates a new module for clock-related classes (currently just
logical and hybrid clock). This is in preparation for introducing
a built-in NTP client.

Since these clock-related classes don't depend on anything else in the
server, it makes sense to move them into a new small module.

Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
---
M CMakeLists.txt
M src/kudu/client/client-test.cc
A src/kudu/clock/CMakeLists.txt
R src/kudu/clock/clock.h
R src/kudu/clock/hybrid_clock-test.cc
R src/kudu/clock/hybrid_clock.cc
R src/kudu/clock/hybrid_clock.h
R src/kudu/clock/logical_clock-test.cc
R src/kudu/clock/logical_clock.cc
R src/kudu/clock/logical_clock.h
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/authn_token_expire-itest.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/master/sys_catalog.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
63 files changed, 221 insertions(+), 229 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/7519/5
-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Move clock-related classes to src/kudu/clock
......................................................................


Patch Set 3:

(3 comments)

fixed tidy issues in modified code (some pre-existing that are getting re-flagged due to the rename)

http://gerrit.cloudera.org:8080/#/c/7519/3/src/kudu/tablet/tablet.h
File src/kudu/tablet/tablet.h:

Line 58: class Clock;
> warning: no definition found for 'Clock', but a definition with the same na
Done


http://gerrit.cloudera.org:8080/#/c/7519/3/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:

Line 58: using log::ReadableLogSegment;
> warning: using decl 'ReadableLogSegment' is unused [misc-unused-using-decls
Done


http://gerrit.cloudera.org:8080/#/c/7519/3/src/kudu/tablet/transactions/transaction_driver.h
File src/kudu/tablet/transactions/transaction_driver.h:

Line 24: #include "kudu/consensus/raft_consensus.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7519

to look at the new patch set (#2).

Change subject: Move clock-related classes to src/kudu/clock
......................................................................

Move clock-related classes to src/kudu/clock

This creates a new module for clock-related classes (currently just
logical and hybrid clock). This is in preparation for introducing
a built-in NTP client.

Since these clock-related classes don't depend on anything else in the
server, it makes sense to move them into a new small module.

Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
---
M CMakeLists.txt
M src/kudu/client/client-test.cc
A src/kudu/clock/CMakeLists.txt
R src/kudu/clock/clock.h
R src/kudu/clock/hybrid_clock-test.cc
R src/kudu/clock/hybrid_clock.cc
R src/kudu/clock/hybrid_clock.h
R src/kudu/clock/logical_clock-test.cc
R src/kudu/clock/logical_clock.cc
R src/kudu/clock/logical_clock.h
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/authn_token_expire-itest.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/server/CMakeLists.txt
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
54 files changed, 127 insertions(+), 124 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/7519/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.

Change subject: Move clock-related classes to src/kudu/clock
......................................................................


Move clock-related classes to src/kudu/clock

This creates a new module for clock-related classes (currently just
logical and hybrid clock). This is in preparation for introducing
a built-in NTP client.

Since these clock-related classes don't depend on anything else in the
server, it makes sense to move them into a new small module.

Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Reviewed-on: http://gerrit.cloudera.org:8080/7519
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M CMakeLists.txt
M src/kudu/client/client-test.cc
A src/kudu/clock/CMakeLists.txt
R src/kudu/clock/clock.h
R src/kudu/clock/hybrid_clock-test.cc
R src/kudu/clock/hybrid_clock.cc
R src/kudu/clock/hybrid_clock.h
R src/kudu/clock/logical_clock-test.cc
R src/kudu/clock/logical_clock.cc
R src/kudu/clock/logical_clock.h
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/authn_token_expire-itest.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/master/sys_catalog.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
63 files changed, 231 insertions(+), 242 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: Move clock-related classes to src/kudu/clock
......................................................................


Patch Set 6: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7519

to look at the new patch set (#4).

Change subject: Move clock-related classes to src/kudu/clock
......................................................................

Move clock-related classes to src/kudu/clock

This creates a new module for clock-related classes (currently just
logical and hybrid clock). This is in preparation for introducing
a built-in NTP client.

Since these clock-related classes don't depend on anything else in the
server, it makes sense to move them into a new small module.

Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
---
M CMakeLists.txt
M src/kudu/client/client-test.cc
A src/kudu/clock/CMakeLists.txt
R src/kudu/clock/clock.h
R src/kudu/clock/hybrid_clock-test.cc
R src/kudu/clock/hybrid_clock.cc
R src/kudu/clock/hybrid_clock.h
R src/kudu/clock/logical_clock-test.cc
R src/kudu/clock/logical_clock.cc
R src/kudu/clock/logical_clock.h
M src/kudu/consensus/CMakeLists.txt
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus_peers-test.cc
M src/kudu/consensus/consensus_queue-test.cc
M src/kudu/consensus/log-test-base.h
M src/kudu/consensus/log_cache-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/consensus/time_manager.cc
M src/kudu/consensus/time_manager.h
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/integration-tests/authn_token_expire-itest.cc
M src/kudu/integration-tests/consistency-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/linked_list-test-util.h
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/integration-tests/ts_recovery-itest.cc
M src/kudu/master/CMakeLists.txt
M src/kudu/master/sys_catalog.cc
M src/kudu/server/CMakeLists.txt
M src/kudu/server/generic_service.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/tablet/CMakeLists.txt
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/major_delta_compaction-test.cc
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/tablet-harness.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_bootstrap.h
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tablet/tablet_replica-test.cc
M src/kudu/tablet/tablet_replica.cc
M src/kudu/tablet/tablet_replica.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tserver/CMakeLists.txt
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
63 files changed, 220 insertions(+), 221 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/7519/4
-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] Move clock-related classes to src/kudu/clock

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: Move clock-related classes to src/kudu/clock
......................................................................


Patch Set 5:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7519/4/src/kudu/clock/CMakeLists.txt
File src/kudu/clock/CMakeLists.txt:

PS4, Line 19:   hybrid_clock.cc
            :   logical_clock.cc)
> Nit: indentation
Done


http://gerrit.cloudera.org:8080/#/c/7519/4/src/kudu/clock/hybrid_clock.h
File src/kudu/clock/hybrid_clock.h:

Line 17: #pragma once
> Mind updating the rest of clock/* to use this?
Done


http://gerrit.cloudera.org:8080/#/c/7519/4/src/kudu/consensus/log-test-base.h
File src/kudu/consensus/log-test-base.h:

Line 62: using consensus::NO_OP;
> Nit: resort
Done


http://gerrit.cloudera.org:8080/#/c/7519/3/src/kudu/integration-tests/consistency-itest.cc
File src/kudu/integration-tests/consistency-itest.cc:

Line 61: using kudu::clock::HybridClock;
> Nit: resort
Done


http://gerrit.cloudera.org:8080/#/c/7519/3/src/kudu/integration-tests/ts_recovery-itest.cc
File src/kudu/integration-tests/ts_recovery-itest.cc:

PS3, Line 62: using clock::Clock;
            : using clock::HybridClock;
> Nit: resort
Done


http://gerrit.cloudera.org:8080/#/c/7519/5/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:

PS5, Line 59: using clock::Clock;
            : using clock::LogicalClock;
> PS4 had the right sort order.
woops... what did I do... thanks


-- 
To view, visit http://gerrit.cloudera.org:8080/7519
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id4f49be6944ec636387f0c9f5721355402e51fc2
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes