You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2020/05/07 01:51:40 UTC

[kudu-CR] tablet: rename "transactions" to "ops" 

Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15870


Change subject: tablet: rename "transactions" to "ops"
......................................................................

tablet: rename "transactions" to "ops"

This renames classes/methods that match the following to use "op"
terminology instead, to disambiguate between a future implementation of
multi-op transactions:
- TransactionState -> OpState
- TransactionDriver -> OpDriver
- TransactionStatus -> OpStatus
- TransactionComple* -> OpComple*
- TransactionTracker -> OpTracker
- InFlightTransaction -> InFlightOp
- PendingTransaction -> PendingOp
- TransactionOrderVerifier -> OpOrderVerifier

This focuses mostly on op lifecycle tracking, and class/method names
only. Further patches will clean up other usages and variable names.

Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
---
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/pending_rounds.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/lock_manager-test.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/lock_manager.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.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/transaction_order_verifier.cc
M src/kudu/tablet/transaction_order_verifier.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.cc
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
35 files changed, 354 insertions(+), 354 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>

[kudu-CR] replace "transaction" terminology with "op"

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: replace "transaction" terminology with "op"
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 13 May 2020 00:03:44 +0000
Gerrit-HasComments: No

[kudu-CR] tablet: update "transaction" methods to use "op" terminology

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: tablet: update "transaction" methods to use "op" terminology
......................................................................


Patch Set 2:

> Patch Set 2:
> 
> > Patch Set 2:
> > 
> > (3 comments)
> > 
> > Hm, I'm thinking of squashing these all into a single commit, given spreading across multiple may make it confusing for reviewers.
> 
> Should be fine.
> 
> One downside of these renaming changes is that it makes backporting non-trivial as auto merge may not go through in many cases.

That's true, but I'd rather pay that cost over compromising clarity moving forward. Hopefully the conflicts would be easy to spot though.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 08 May 2020 22:22:22 +0000
Gerrit-HasComments: No

[kudu-CR] replace "transaction" terminology with "op"

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

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

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

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

Change subject: replace "transaction" terminology with "op"
......................................................................

replace "transaction" terminology with "op"

This renames most "transaction" terminology found in class names, method
names, and comments to use "op" instead, in preparation for a future
implementation of multi-op transactions.

Also a stray tx -> ts in the context of printing delta keys, which are
(row idx, timestamp).

I've left filenames, metrics, and gflags in tact. Future patches will
address these.

Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
---
M src/kudu/client/client.h
M src/kudu/client/client.proto
M src/kudu/clock/clock.h
M src/kudu/common/common.proto
M src/kudu/common/timestamp.h
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/pending_rounds.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-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/delete_table-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/raft_consensus_election-itest.cc
M src/kudu/integration-tests/time_anomalies-itest.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/server/server_base.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_key.h
M src/kudu/tablet/delta_relevancy.h
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/lock_manager-test.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/lock_manager.h
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mutation.h
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_random_access-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/transaction_order_verifier.cc
M src/kudu/tablet/transaction_order_verifier.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.cc
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
86 files changed, 1,993 insertions(+), 2,012 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] replace "transaction" terminology with "op"

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

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

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

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

Change subject: replace "transaction" terminology with "op"
......................................................................

replace "transaction" terminology with "op"

This renames most "transaction" terminology found in class names, method
names, and comments to use "op" instead, in preparation for a future
implementation of multi-op transactions.

Also a stray tx -> ts in the context of printing delta keys, which are
(row idx, timestamp).

I've left filenames, metrics, and gflags in tact. Future patches will
address these.

Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
---
M src/kudu/client/client.h
M src/kudu/client/client.proto
M src/kudu/clock/clock.h
M src/kudu/common/common.proto
M src/kudu/common/timestamp.h
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/pending_rounds.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-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/delete_table-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/raft_consensus_election-itest.cc
M src/kudu/integration-tests/time_anomalies-itest.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/server/server_base.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_key.h
M src/kudu/tablet/delta_relevancy.h
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/lock_manager-test.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/lock_manager.h
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mutation.h
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_random_access-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/transaction_order_verifier.cc
M src/kudu/tablet/transaction_order_verifier.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.cc
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
86 files changed, 1,998 insertions(+), 2,017 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] replace "transaction" terminology with "op"

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

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

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

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

Change subject: replace "transaction" terminology with "op"
......................................................................

replace "transaction" terminology with "op"

This renames most "transaction" terminology found in class names, method
names, and comments to use "op" instead, in preparation for a future
implementation of multi-op transactions.

Also a stray tx -> ts in the context of printing delta keys, which are
(row idx, timestamp).

I've left filenames, metrics, and gflags in tact. Future patches will
address these.

Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
---
M src/kudu/client/client.h
M src/kudu/client/client.proto
M src/kudu/clock/clock.h
M src/kudu/common/common.proto
M src/kudu/common/timestamp.h
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/pending_rounds.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-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/delete_table-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/raft_consensus_election-itest.cc
M src/kudu/integration-tests/time_anomalies-itest.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/server/server_base.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_key.h
M src/kudu/tablet/delta_relevancy.h
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/lock_manager-test.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/lock_manager.h
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mutation.h
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_random_access-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/transaction_order_verifier.cc
M src/kudu/tablet/transaction_order_verifier.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.cc
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
86 files changed, 1,998 insertions(+), 2,017 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] replace "transaction" terminology with "op"

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: replace "transaction" terminology with "op"
......................................................................


Patch Set 5:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/consensus.proto@99
PS3, Line 99: a
> an
Done


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/raft_consensus.h@872
PS3, Line 872: a
> an
Done


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/raft_consensus.h@987
PS3, Line 987: the no-op 
> This is a bit confusing. What does non-op op mean?
Done

This is referring to the no-op round that must succeed upon becoming a leader.


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/time_manager.cc
File src/kudu/consensus/time_manager.cc:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/time_manager.cc@322
PS3, Line 322: be an
> be an op
Done


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

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/compaction-test.cc@156
PS3, Line 156: MemRowSet *mrs,
             :                      const ScopedOp& op,
             :                      int row_key,
             :                      int32_t val) {
> Indentation
Done


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/tablet.cc@1232
PS3, Line 1232: AlterSchemaOpState* 
> * can be sticked to datatype.
Done


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

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/transactions/transaction_driver.cc@104
PS3, Line 104: OpDriver::OpDriver(OpTracker *op_tracker,
             :                    RaftConsensus* consensus,
             :                    Log* log,
             :                    ThreadPoolToken* prepare_pool_token,
             :                    ThreadPool* apply_pool,
             :                    OpOrderVerifier* order_verifier)
> Indentation
Done


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/transactions/transaction_driver.cc@550
PS3, Line 550: OpState* op_state,
             :                                     const Timestamp& timestamp) {
> Indentation
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 12 May 2020 23:47:12 +0000
Gerrit-HasComments: Yes

[kudu-CR] tablet: update "transaction" methods to use "op" terminology

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: tablet: update "transaction" methods to use "op" terminology
......................................................................


Patch Set 2:

> Patch Set 2:
> 
> (3 comments)
> 
> Hm, I'm thinking of squashing these all into a single commit, given spreading across multiple may make it confusing for reviewers.

Should be fine.

One downside of these renaming changes is that it makes backporting non-trivial as auto merge may not go through in many cases.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 08 May 2020 20:35:49 +0000
Gerrit-HasComments: No

[kudu-CR] replace "transaction" terminology with "op"

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: replace "transaction" terminology with "op"
......................................................................


Patch Set 3:

(5 comments)

Reviewed about half-way...

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/consensus.proto@99
PS3, Line 99: a
an


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/raft_consensus.h
File src/kudu/consensus/raft_consensus.h:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/raft_consensus.h@872
PS3, Line 872: a
an


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/raft_consensus.h@987
PS3, Line 987: non-op ops
This is a bit confusing. What does non-op op mean?


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/time_manager.cc
File src/kudu/consensus/time_manager.cc:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/consensus/time_manager.cc@322
PS3, Line 322: be op
be an op


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

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/compaction-test.cc@156
PS3, Line 156: MemRowSet *mrs,
             :                               const ScopedOp& op,
             :                               int row_key,
             :                               int32_t val) {
Indentation



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 12 May 2020 23:17:19 +0000
Gerrit-HasComments: Yes

[kudu-CR] tablet: update "transaction" methods to use "op" terminology

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: tablet: update "transaction" methods to use "op" terminology
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/consensus.proto@417
PS2, Line 417: transaction.
operation?


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/pending_rounds.h
File src/kudu/consensus/pending_rounds.h:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/pending_rounds.h@75
PS2, Line 75: transaction
This and other places below.


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/raft_consensus.cc@1463
PS2, Line 1463: transactions
Same here


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/master/sys_catalog.cc@489
PS2, Line 489:       new tablet::WriteOpState(tablet_replica_.get(),
             :                                         &req,
             :                                         nullptr, // No RequestIdPB
             :                                         &resp));
Alignment here and bunch of places below.

This is where using an IDE would help as it would automatically adjust alignment as well as part of refactoring.


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/tablet/tablet_replica.h@224
PS2, Line 224: Transaction::TraceType trace_type,
             :                                std::vector<consensus::OpStatusPB>* out)
Alignment


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/tablet/tablet_replica.h@258
PS2, Line 258: Status NewLeaderOpDriver(std::unique_ptr<Transaction> transaction,
             :                                     scoped_refptr<OpDriver>* driver);
             : 
             :   Status NewReplicaOpDriver(std::unique_ptr<Transaction> transaction,
             :                                      scoped_refptr<OpDriver>* driver);
Alignment



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 08 May 2020 17:32:52 +0000
Gerrit-HasComments: Yes

[kudu-CR] tablet: rename "transactions" to "ops"

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: tablet: rename "transactions" to "ops"
......................................................................


Patch Set 1:

woohoo! i'm in favor of this patch series (though I didn't look at the patch so I won't be the one to +2). Do you have a plan for what to do with the metrics? Rename them or leave them as is?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 07 May 2020 03:39:26 +0000
Gerrit-HasComments: No

[kudu-CR] replace "transaction" terminology with "op"

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

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

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

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

Change subject: replace "transaction" terminology with "op"
......................................................................

replace "transaction" terminology with "op"

This renames most "transaction" terminology found in class names, method
names, and comments to use "op" instead, in preparation for a future
implementation of multi-op transactions.

Also a stray tx -> ts in the context of printing delta keys, which are
(row idx, timestamp).

I've left filenames, metrics, and gflags in tact. Future patches will
address these.

Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
---
M src/kudu/client/client.proto
M src/kudu/common/common.proto
M src/kudu/common/timestamp.h
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/log.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/pending_rounds.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-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/delete_table-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/raft_consensus_election-itest.cc
M src/kudu/integration-tests/time_anomalies-itest.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/server/server_base.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_key.h
M src/kudu/tablet/delta_relevancy.h
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/lock_manager-test.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/lock_manager.h
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mutation.h
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_random_access-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/transaction_order_verifier.cc
M src/kudu/tablet/transaction_order_verifier.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.cc
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
82 files changed, 1,951 insertions(+), 1,969 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] replace "transaction" terminology with "op"

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: replace "transaction" terminology with "op"
......................................................................

replace "transaction" terminology with "op"

This renames most "transaction" terminology found in class names, method
names, and comments to use "op" instead, in preparation for a future
implementation of multi-op transactions.

Also a stray tx -> ts in the context of printing delta keys, which are
(row idx, timestamp).

I've left filenames, metrics, and gflags in tact. Future patches will
address these.

Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Reviewed-on: http://gerrit.cloudera.org:8080/15870
Reviewed-by: Bankim Bhavsar <ba...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/client/client.h
M src/kudu/client/client.proto
M src/kudu/clock/clock.h
M src/kudu/common/common.proto
M src/kudu/common/timestamp.h
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/log.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/pending_rounds.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-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/delete_table-itest.cc
M src/kudu/integration-tests/exactly_once_writes-itest.cc
M src/kudu/integration-tests/fuzz-itest.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/raft_consensus_election-itest.cc
M src/kudu/integration-tests/time_anomalies-itest.cc
M src/kudu/kserver/kserver.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/server/server_base.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
M src/kudu/tablet/delta_compaction.cc
M src/kudu/tablet/delta_key.h
M src/kudu/tablet/delta_relevancy.h
M src/kudu/tablet/delta_stats.h
M src/kudu/tablet/delta_store.cc
M src/kudu/tablet/delta_store.h
M src/kudu/tablet/deltafile-test.cc
M src/kudu/tablet/deltamemstore-test.cc
M src/kudu/tablet/diff_scan-test.cc
M src/kudu/tablet/diskrowset-test-base.h
M src/kudu/tablet/diskrowset-test.cc
M src/kudu/tablet/diskrowset.cc
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/lock_manager-test.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/lock_manager.h
M src/kudu/tablet/memrowset-test.cc
M src/kudu/tablet/memrowset.h
M src/kudu/tablet/mutation.h
M src/kudu/tablet/mvcc-test.cc
M src/kudu/tablet/mvcc.cc
M src/kudu/tablet/mvcc.h
M src/kudu/tablet/rowset.cc
M src/kudu/tablet/rowset.h
M src/kudu/tablet/tablet-test-base.h
M src/kudu/tablet/tablet-test-util.h
M src/kudu/tablet/tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet.h
M src/kudu/tablet/tablet.proto
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tablet/tablet_random_access-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/transaction_order_verifier.cc
M src/kudu/tablet/transaction_order_verifier.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.cc
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/tserver.proto
M src/kudu/tserver/tserver_path_handlers.cc
86 files changed, 1,998 insertions(+), 2,017 deletions(-)

Approvals:
  Bankim Bhavsar: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] tablet: update "transaction" methods to use "op" terminology

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: tablet: update "transaction" methods to use "op" terminology
......................................................................


Patch Set 2:

(3 comments)

Hm, I'm thinking of squashing these all into a single commit, given spreading across multiple may make it confusing for reviewers.

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/consensus.proto
File src/kudu/consensus/consensus.proto:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/consensus.proto@417
PS2, Line 417: transaction.
> operation?
Most comments are addressed in a different patch https://gerrit.cloudera.org/c/15873/2


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/pending_rounds.h
File src/kudu/consensus/pending_rounds.h:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/pending_rounds.h@75
PS2, Line 75: transaction
> This and other places below.
Ack


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/consensus/raft_consensus.cc@1463
PS2, Line 1463: transactions
> Same here
Ack



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 08 May 2020 18:49:11 +0000
Gerrit-HasComments: Yes

[kudu-CR] replace "transaction" terminology with "op"

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: replace "transaction" terminology with "op"
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/tablet.cc@1232
PS3, Line 1232: AlterSchemaOpState *
* can be sticked to datatype.


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

http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/transactions/transaction_driver.cc@104
PS3, Line 104: OpDriver::OpDriver(OpTracker *op_tracker,
             :                                      RaftConsensus* consensus,
             :                                      Log* log,
             :                                      ThreadPoolToken* prepare_pool_token,
             :                                      ThreadPool* apply_pool,
             :                                      OpOrderVerifier* order_verifier)
Indentation


http://gerrit.cloudera.org:8080/#/c/15870/3/src/kudu/tablet/transactions/transaction_driver.cc@550
PS3, Line 550: OpState* op_state,
             :                                              const Timestamp& timestamp) {
Indentation



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 12 May 2020 23:34:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] tablet: rename "transactions" to "ops"

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: tablet: rename "transactions" to "ops"
......................................................................


Patch Set 1:

> Patch Set 1:
> 
> woohoo! i'm in favor of this patch series (though I didn't look at the patch so I won't be the one to +2). Do you have a plan for what to do with the metrics? Rename them or leave them as is?

I don't have a concrete plan yet. For the time being I'm thinking of leaving them as is, until more of the implementation materialized.

I think at worst, we'd just rename them and release note the change. Alternatively, we might consider building out some kind of aliasing mechanism that publishes both 'txn' and 'op' variants, but tracks a single value.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 07 May 2020 04:27:59 +0000
Gerrit-HasComments: No

[kudu-CR] replace "transaction" terminology with "op"

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15870 )

Change subject: replace "transaction" terminology with "op"
......................................................................


Patch Set 3:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/master/sys_catalog.cc@489
PS2, Line 489:       new tablet::WriteOpState(tablet_replica_.get(),
             :                                &req,
             :                                nullptr, // No RequestIdPB
             :                                &resp));
> Alignment here and bunch of places below.
Done

Fair point about IDEs, I may try that on the next big rename (maybe cfile::"block" -> cfile::"page", to avoid conflicts with fs::"block").


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/tablet/tablet_replica.h
File src/kudu/tablet/tablet_replica.h:

http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/tablet/tablet_replica.h@224
PS2, Line 224: Op::TraceType trace_type,
             :                       std::vector<consensus::OpStatusPB>* out) const;
> Alignment
Done


http://gerrit.cloudera.org:8080/#/c/15870/2/src/kudu/tablet/tablet_replica.h@258
PS2, Line 258: Status NewLeaderOpDriver(std::unique_ptr<Op> transaction,
             :                            scoped_refptr<OpDriver>* driver);
             : 
             :   Status NewReplicaOpDriver(std::unique_ptr<Op> transaction,
             :                             scoped_refptr<OpDriver>* driver);
> Alignment
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Sat, 09 May 2020 01:19:25 +0000
Gerrit-HasComments: Yes

[kudu-CR] tablet: update "transaction" methods to use "op" terminology

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

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

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

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

Change subject: tablet: update "transaction" methods to use "op" terminology
......................................................................

tablet: update "transaction" methods to use "op" terminology

This renames classes/methods that match the following to use "op"
terminology instead, to disambiguate between a future implementation of
multi-op transactions:
- TransactionState -> OpState
- TransactionDriver -> OpDriver
- TransactionStatus -> OpStatus
- TransactionComple* -> OpComple*
- TransactionTracker -> OpTracker
- InFlightTransaction -> InFlightOp
- PendingTransaction -> PendingOp
- TransactionOrderVerifier -> OpOrderVerifier

This focuses mostly on op lifecycle tracking, and class/method names
only. Further patches will clean up other usages and variable names.

Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
---
M src/kudu/consensus/consensus-test-util.h
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/pending_rounds.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/tablet/local_tablet_writer.h
M src/kudu/tablet/lock_manager-test.cc
M src/kudu/tablet/lock_manager.cc
M src/kudu/tablet/lock_manager.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.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/transaction_order_verifier.cc
M src/kudu/tablet/transaction_order_verifier.h
M src/kudu/tablet/transactions/alter_schema_transaction.cc
M src/kudu/tablet/transactions/alter_schema_transaction.h
M src/kudu/tablet/transactions/transaction.cc
M src/kudu/tablet/transactions/transaction.h
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tablet/transactions/transaction_driver.h
M src/kudu/tablet/transactions/transaction_tracker-test.cc
M src/kudu/tablet/transactions/transaction_tracker.cc
M src/kudu/tablet/transactions/transaction_tracker.h
M src/kudu/tablet/transactions/write_transaction.cc
M src/kudu/tablet/transactions/write_transaction.h
M src/kudu/tserver/tablet_copy_source_session-test.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tserver_path_handlers.cc
35 files changed, 354 insertions(+), 354 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I86df8b51e1e8fdf702b79486486b402465476ad5
Gerrit-Change-Number: 15870
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>