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 2020/12/17 07:22:47 UTC
[kudu-CR] [client] KUDU-2612: two more scenarios for tnx keepalive
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16886
Change subject: [client] KUDU-2612: two more scenarios for tnx keepalive
......................................................................
[client] KUDU-2612: two more scenarios for tnx keepalive
This patch adds two more test scenarios related to txn keepalive
messages automatically sent by Kudu C++ client. These are to verify
the corresponding functionality of the client when TxnManager instances
are not available for short and long time intervals, where 'short' and
'long' are relative to the txn keepalive heartbeat timeout interval.
In addition, I sneaked in one small fix to avoid TSAN warnings
when running the newly introduced tests: the removed CHECK() in
Master::WaitForTxnManagerInit() isn't crucial.
Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
---
M src/kudu/client/client-test.cc
M src/kudu/master/master.cc
2 files changed, 95 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/16886/1
--
To view, visit http://gerrit.cloudera.org:8080/16886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Gerrit-Change-Number: 16886
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] KUDU-2612: two more scenarios for txn keepalive in client
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16886 )
Change subject: KUDU-2612: two more scenarios for txn keepalive in client
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc@7661
PS2, Line 7661:
nit: skip if slow tests not allowed?
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc@7669
PS2, Line 7669: FLAGS_txn_keepalive_interval_ms
Should we shorten this, given the default value is 30s?
--
To view, visit http://gerrit.cloudera.org:8080/16886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Gerrit-Change-Number: 16886
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 18 Dec 2020 01:09:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: two more scenarios for txn keepalive in client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16886 )
Change subject: KUDU-2612: two more scenarios for txn keepalive in client
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc@7661
PS2, Line 7661:
> nit: skip if slow tests not allowed?
This test runs pretty fast given FLAGS_txn_keepalive_interval_ms is set to 250 in ClientTest::SetUp().
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc@7669
PS2, Line 7669: FLAGS_txn_keepalive_interval_ms
> Should we shorten this, given the default value is 30s?
Nope, I don't think so: the value of --txn_keepalive_interval_ms in this test is 205 ms (it's set in ClientTest::SetUp()).
--
To view, visit http://gerrit.cloudera.org:8080/16886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Gerrit-Change-Number: 16886
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: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 18 Dec 2020 04:44:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: two more scenarios for txn keepalive in client
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16886 )
Change subject: KUDU-2612: two more scenarios for txn keepalive in client
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc@7661
PS2, Line 7661:
> This test runs pretty fast given FLAGS_txn_keepalive_interval_ms is set to
Ah right. For some reason I assumed defaults.
--
To view, visit http://gerrit.cloudera.org:8080/16886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Gerrit-Change-Number: 16886
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: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 18 Dec 2020 05:52:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: two more scenarios for txn keepalive in client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16886
to look at the new patch set (#2).
Change subject: KUDU-2612: two more scenarios for txn keepalive in client
......................................................................
KUDU-2612: two more scenarios for txn keepalive in client
This patch adds two more test scenarios related to txn keepalive
messages automatically sent by Kudu C++ client. These are to verify
the corresponding functionality of the client when TxnManager instances
are not available for short and long time intervals, where 'short' and
'long' are relative to the txn keepalive heartbeat timeout interval.
In addition, I sneaked in one small fix to avoid TSAN warnings
when running the newly introduced tests: the removed CHECK() in
Master::WaitForTxnManagerInit() isn't crucial.
Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
---
M src/kudu/client/client-test.cc
M src/kudu/master/master.cc
2 files changed, 95 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/16886/2
--
To view, visit http://gerrit.cloudera.org:8080/16886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Gerrit-Change-Number: 16886
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612: two more scenarios for txn keepalive in client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16886 )
Change subject: KUDU-2612: two more scenarios for txn keepalive in client
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/16886/2/src/kudu/client/client-test.cc@7661
PS2, Line 7661:
> Ah right. For some reason I assumed defaults.
Indeed, that's not obvious if just looking at this diff.
Not sure what's the best way to make it more obvious: creating a sub-class ClientTxnTest of ClientTest for txn-related scenarios and make the override only in ClientTxnTest::SetUp() or add a comment into each txn scenario about the override in ClientTest::SetUp()?
I think I'll leave this as is for a while. Meanwhile, let me know what do you think will help to make this better from the readability perspective -- I'll address it in a separate changelist.
Thank you!
--
To view, visit http://gerrit.cloudera.org:8080/16886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Gerrit-Change-Number: 16886
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: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 18 Dec 2020 06:51:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: two more scenarios for txn keepalive in 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/16886 )
Change subject: KUDU-2612: two more scenarios for txn keepalive in client
......................................................................
KUDU-2612: two more scenarios for txn keepalive in client
This patch adds two more test scenarios related to txn keepalive
messages automatically sent by Kudu C++ client. These are to verify
the corresponding functionality of the client when TxnManager instances
are not available for short and long time intervals, where 'short' and
'long' are relative to the txn keepalive heartbeat timeout interval.
In addition, I sneaked in one small fix to avoid TSAN warnings
when running the newly introduced tests: the removed CHECK() in
Master::WaitForTxnManagerInit() isn't crucial.
Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Reviewed-on: http://gerrit.cloudera.org:8080/16886
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/client/client-test.cc
M src/kudu/master/master.cc
2 files changed, 95 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16886
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I42988996f1ddb1cd456d289ea7e15586bd7e3dc7
Gerrit-Change-Number: 16886
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: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)