You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2018/07/27 00:49:17 UTC

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

David Ribeiro Alves has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11070


Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................

WIP: Atomicize OpId and Timestamp assignment

This is a simpler version of a previous patch but without big
changes to the queue or consensus.

Change-Id: I0d369423dd5c96b653b424c29744507edd874357
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/raft_consensus.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/raft_consensus-itest.cc
M src/kudu/integration-tests/test_workload.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
13 files changed, 86 insertions(+), 85 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 1
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

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

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................


Patch Set 8: Verified+1

flakes are unrelated:
- ToolTest.TestTserverList
- org.apache.kudu.spark.kudu.DefaultSourceTest.socket read timeout propagation


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 8
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 01 Aug 2018 00:20:48 +0000
Gerrit-HasComments: No

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................

WIP: Atomicize OpId and Timestamp assignment

This is a simpler version of a previous patch but without big
changes to the queue or consensus.

Change-Id: I0d369423dd5c96b653b424c29744507edd874357
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tserver/tablet_server-test.cc
7 files changed, 75 insertions(+), 31 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 6
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................

WIP: Atomicize OpId and Timestamp assignment

This is a simpler version of a previous patch but without big
changes to the queue or consensus.

Change-Id: I0d369423dd5c96b653b424c29744507edd874357
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
8 files changed, 77 insertions(+), 32 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 4
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................

WIP: Atomicize OpId and Timestamp assignment

This is a simpler version of a previous patch but without big
changes to the queue or consensus.

Change-Id: I0d369423dd5c96b653b424c29744507edd874357
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tserver/tablet_server-test.cc
7 files changed, 74 insertions(+), 31 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 5
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................

WIP: Atomicize OpId and Timestamp assignment

This is a simpler version of a previous patch but without big
changes to the queue or consensus.

Change-Id: I0d369423dd5c96b653b424c29744507edd874357
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/raft_consensus.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/tablet/transactions/transaction_driver.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
11 files changed, 85 insertions(+), 84 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 2
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................

WIP: Atomicize OpId and Timestamp assignment

This is a simpler version of a previous patch but without big
changes to the queue or consensus.

Change-Id: I0d369423dd5c96b653b424c29744507edd874357
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/time_manager-test.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/tserver/ts_tablet_manager.cc
9 files changed, 77 insertions(+), 41 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has uploaded a new patch set (#7) to the change originally created by David Ribeiro Alves. ( http://gerrit.cloudera.org:8080/11070 )

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................

WIP: Atomicize OpId and Timestamp assignment

This is a simpler version of a previous patch but without big
changes to the queue or consensus.

Change-Id: I0d369423dd5c96b653b424c29744507edd874357
---
M java/kudu-client/src/test/java/org/apache/kudu/client/TestHybridTime.java
M src/kudu/consensus/consensus_meta-test.cc
M src/kudu/consensus/pending_rounds.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/integration-tests/alter_table-test.cc
M src/kudu/tablet/transactions/transaction_driver.cc
M src/kudu/tserver/tablet_server-test.cc
7 files changed, 74 insertions(+), 31 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 7
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/11070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 8
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] WIP: Atomicize OpId and Timestamp assignment

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

Change subject: WIP: Atomicize OpId and Timestamp assignment
......................................................................


Patch Set 8:

(1 comment)

Noting the root of our timeouts.

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

http://gerrit.cloudera.org:8080/#/c/11070/8/src/kudu/tablet/transactions/transaction_driver.cc@354
PS8, Line 354: return s;
The changes from PS6 to PS8 are due to the fact that:
- returning any sort of error will have HandleFailure called on it externally by the TransactionDriver::ExecuteAsync()
- if the ReplicationFinished() callback is called between Replicate() and L372, the replication state copy may be REPLICATED, in which case ApplyAsync applies the txn, or REPLICATION_FAILED (eg because the tablet is shutting down), in which case ApplyAsync handles the error, unregistering the txn from the tracker

This meant that if ReplicationFinished() was called but set the replication state to REPLICATION_FAILED, we were left with a transaction on the transaction tracker, and upon waiting for all transactions to finish (e.g. when stopping / deleting the tablet), we would wait forever.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0d369423dd5c96b653b424c29744507edd874357
Gerrit-Change-Number: 11070
Gerrit-PatchSet: 8
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 01 Aug 2018 00:19:54 +0000
Gerrit-HasComments: Yes