You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2018/04/11 23:30:01 UTC
[kudu-CR] HmsCatalog: fix bug in reconnection backoff logic
Hello Adar Dembo, Hao Hao,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10028
to review the following change.
Change subject: HmsCatalog: fix bug in reconnection backoff logic
......................................................................
HmsCatalog: fix bug in reconnection backoff logic
This fixes an accidentally shadowed Status variable. The effect of this
bug was that the HmsCatalog would not throttle HMS reconnect attempts.
The tests actually took advantage of this, I've had to update them to
try ops multiple times after an HMS restart.
No tests are included, but I've left a TODO about adding tests when
HmsCatalog stats are introduced.
Change-Id: I726283ec8332d16942c82c73492709de4323a43d
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/integration-tests/master_hms-itest.cc
3 files changed, 29 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/10028/1
--
To view, visit http://gerrit.cloudera.org:8080/10028
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
[kudu-CR] HmsCatalog: fix bug in reconnection backoff logic
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/10028 )
Change subject: HmsCatalog: fix bug in reconnection backoff logic
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10028
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 12 Apr 2018 20:23:37 +0000
Gerrit-HasComments: No
[kudu-CR] HmsCatalog: fix bug in reconnection backoff logic
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10028 )
Change subject: HmsCatalog: fix bug in reconnection backoff logic
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10028/1/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/10028/1/src/kudu/integration-tests/master_hms-itest.cc@279
PS1, Line 279: // HmsCatalog throttles reconnections, so it's necessary to wait out the backoff.
: table_alterer.reset(client_->NewTableAlterer("db.c"));
: ASSERT_OK(table_alterer->RenameTo("db.a")->Alter());
Do we have to reconstruct the TableAlterer each time? Couldn't we just call Alter() in the body of the ASSERT_EVENTUALLY?
Below too.
--
To view, visit http://gerrit.cloudera.org:8080/10028
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 12 Apr 2018 00:14:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] HmsCatalog: fix bug in reconnection backoff logic
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10028 )
Change subject: HmsCatalog: fix bug in reconnection backoff logic
......................................................................
HmsCatalog: fix bug in reconnection backoff logic
This fixes an accidentally shadowed Status variable. The effect of this
bug was that the HmsCatalog would not throttle HMS reconnect attempts.
The tests actually took advantage of this, I've had to update them to
try ops multiple times after an HMS restart.
No tests are included, but I've left a TODO about adding tests when
HmsCatalog stats are introduced.
Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Reviewed-on: http://gerrit.cloudera.org:8080/10028
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/integration-tests/master_hms-itest.cc
3 files changed, 31 insertions(+), 18 deletions(-)
Approvals:
Kudu Jenkins: Verified
Hao Hao: Looks good to me, approved
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10028
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] HmsCatalog: fix bug in reconnection backoff logic
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10028
to look at the new patch set (#2).
Change subject: HmsCatalog: fix bug in reconnection backoff logic
......................................................................
HmsCatalog: fix bug in reconnection backoff logic
This fixes an accidentally shadowed Status variable. The effect of this
bug was that the HmsCatalog would not throttle HMS reconnect attempts.
The tests actually took advantage of this, I've had to update them to
try ops multiple times after an HMS restart.
No tests are included, but I've left a TODO about adding tests when
HmsCatalog stats are introduced.
Change-Id: I726283ec8332d16942c82c73492709de4323a43d
---
M src/kudu/hms/hms_catalog-test.cc
M src/kudu/hms/hms_catalog.cc
M src/kudu/integration-tests/master_hms-itest.cc
3 files changed, 31 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/10028/2
--
To view, visit http://gerrit.cloudera.org:8080/10028
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] HmsCatalog: fix bug in reconnection backoff logic
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10028 )
Change subject: HmsCatalog: fix bug in reconnection backoff logic
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10028
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 12 Apr 2018 21:21:12 +0000
Gerrit-HasComments: No
[kudu-CR] HmsCatalog: fix bug in reconnection backoff logic
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/10028 )
Change subject: HmsCatalog: fix bug in reconnection backoff logic
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10028/1/src/kudu/integration-tests/master_hms-itest.cc
File src/kudu/integration-tests/master_hms-itest.cc:
http://gerrit.cloudera.org:8080/#/c/10028/1/src/kudu/integration-tests/master_hms-itest.cc@279
PS1, Line 279: // HmsCatalog throttles reconnections, so it's necessary to wait out the backoff.
: ASSERT_OK(table_alterer->Alter());
: });
> Do we have to reconstruct the TableAlterer each time? Couldn't we just call
Done
--
To view, visit http://gerrit.cloudera.org:8080/10028
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I726283ec8332d16942c82c73492709de4323a43d
Gerrit-Change-Number: 10028
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 12 Apr 2018 19:04:10 +0000
Gerrit-HasComments: Yes