You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2021/04/27 04:29:30 UTC

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client) 

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17345


Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................

KUDU-2612: propagate commit timestamp (C++ client)

With this patch, the commit timestamp for a non-empty transaction
is propagated to a Kudu C++ client upon calling
KuduTransaction::IsCommitComplete() (that means that committing
a transaction synchronously via KuduTransaction::Commit() propagates
the commit timestamp as well).

Updating the last observed timestamp with the commit timestamp is
necessary to achieve consistency in the READ_YOUR_WRITES mode when
reading the data of a transaction which has just been committed.  The
commit phase might take some time and may even be retried in some cases,
so even the client observed timestamps for all the write operations
it sent in the context this transaction, the maximum timestamp collected
among the involved transaction participants might be far ahead of the
last timestamp observed by the client so far.

Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
---
M src/kudu/client/transaction-internal.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/master/txn_manager.proto
M src/kudu/master/txn_manager_service.cc
M src/kudu/transactions/txn_status_manager.cc
M src/kudu/transactions/txn_system_client.cc
6 files changed, 143 insertions(+), 12 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/integration-tests/txn_write_ops-itest.cc
File src/kudu/integration-tests/txn_write_ops-itest.cc:

http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/integration-tests/txn_write_ops-itest.cc@314
PS1, Line 314:   //NO_FATALS(StartCluster(tserver_flags, master_flags, kNumTabletServers));
> nit: drop?
Done


http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/master/txn_manager.proto
File src/kudu/master/txn_manager.proto:

http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/master/txn_manager.proto@100
PS1, Line 100: FINILIZE_IN_PROGRESS
> nit: FINALIZE_IN_PROGRESS
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 27 Apr 2021 18:07:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................


Patch Set 1: Code-Review+2

(2 comments)

Thanks for making this change!

http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/integration-tests/txn_write_ops-itest.cc
File src/kudu/integration-tests/txn_write_ops-itest.cc:

http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/integration-tests/txn_write_ops-itest.cc@314
PS1, Line 314:   //NO_FATALS(StartCluster(tserver_flags, master_flags, kNumTabletServers));
nit: drop?


http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/master/txn_manager.proto
File src/kudu/master/txn_manager.proto:

http://gerrit.cloudera.org:8080/#/c/17345/1/src/kudu/master/txn_manager.proto@100
PS1, Line 100: FINILIZE_IN_PROGRESS
nit: FINALIZE_IN_PROGRESS



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 27 Apr 2021 18:01:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................

KUDU-2612: propagate commit timestamp (C++ client)

With this patch, the commit timestamp for a non-empty transaction
is propagated to a Kudu C++ client upon calling
KuduTransaction::IsCommitComplete() (that means that committing
a transaction synchronously via KuduTransaction::Commit() propagates
the commit timestamp as well).

Updating the last observed timestamp with the commit timestamp is
necessary to achieve consistency in the READ_YOUR_WRITES mode when
reading the data of a transaction which has just been committed.  The
commit phase might take some time and may even be retried in some cases,
so even the client observed timestamps for all the write operations
it sent in the context this transaction, the maximum timestamp collected
among the involved transaction participants might be far ahead of the
last timestamp observed by the client so far.

Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Reviewed-on: http://gerrit.cloudera.org:8080/17345
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/client/transaction-internal.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/master/txn_manager.proto
M src/kudu/master/txn_manager_service.cc
M src/kudu/transactions/txn_status_manager.cc
M src/kudu/transactions/txn_system_client.cc
6 files changed, 142 insertions(+), 12 deletions(-)

Approvals:
  Andrew Wong: Looks good to me, approved
  Alexey Serbin: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................


Patch Set 1: Verified+1

unrelated flakes in:
  * ToolTest.TestLoadgenKeepAutoTableAndData
  * testClientIdFilter(org.apache.kudu.client.TestKuduMetrics)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 27 Apr 2021 07:06:55 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................


Patch Set 2: Verified+1

unrelated test failures:
  * AuthzProviders/TSAuthzITest.TestReadsAndWrites/Ranger
  * CopyTableParameterized/ToolTestCopyTableParameterized.TestCopyTable/5


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 27 Apr 2021 19:12:16 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 27 Apr 2021 18:29:50 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2612: propagate commit timestamp (C++ client)

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

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

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

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

Change subject: KUDU-2612: propagate commit timestamp (C++ client)
......................................................................

KUDU-2612: propagate commit timestamp (C++ client)

With this patch, the commit timestamp for a non-empty transaction
is propagated to a Kudu C++ client upon calling
KuduTransaction::IsCommitComplete() (that means that committing
a transaction synchronously via KuduTransaction::Commit() propagates
the commit timestamp as well).

Updating the last observed timestamp with the commit timestamp is
necessary to achieve consistency in the READ_YOUR_WRITES mode when
reading the data of a transaction which has just been committed.  The
commit phase might take some time and may even be retried in some cases,
so even the client observed timestamps for all the write operations
it sent in the context this transaction, the maximum timestamp collected
among the involved transaction participants might be far ahead of the
last timestamp observed by the client so far.

Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
---
M src/kudu/client/transaction-internal.cc
M src/kudu/integration-tests/txn_write_ops-itest.cc
M src/kudu/master/txn_manager.proto
M src/kudu/master/txn_manager_service.cc
M src/kudu/transactions/txn_status_manager.cc
M src/kudu/transactions/txn_system_client.cc
6 files changed, 142 insertions(+), 12 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If3ff321895a25326b962a47d5fa868e45aefcb4d
Gerrit-Change-Number: 17345
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)