You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2019/04/15 23:59:11 UTC

[kudu-CR] log block manager: reenable dead container deletion at runtime

Hello Andrew Wong, helifu, Hao Hao,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................

log_block_manager: reenable dead container deletion at runtime

A quick timeline:
1. The feature was added and its behavior enabled.
2. Occasionally, block_manager-stress-test would SIGSEGV [1]. These failures
   showed up in the flaky test dashboard.
3. So, a feature flag was added in commit 8dc7904. The flag was false by
   default, only set to true for a handful of tests.
4. In commit 3e0fcc9, the flag was set to true in block_manager-stress-test
   so that we could continue to debug the issue.
5. Except that it hasn't surfaced since then; the last
   block_manager-stress-test failure was on 02/03/19.

It's a mystery as to why there haven't been any block_manager-stress-test
failures since #4. Maybe something isn't working quite right with the
feature flag, or maybe the failure was due to something else. Either way,
let's try to enable the feature again, since it's still early in the 1.10
release cycle.

1. http://dist-test.cloudera.org:8080/download_log?c195c2b0-2845-11e9-a3f1-0242ac110002

Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
3 files changed, 1 insertion(+), 15 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>

[kudu-CR] log block manager: reenable dead container deletion at runtime

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

Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13021/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13021/1//COMMIT_MSG@26
PS1, Line 26: 1. http://dist-test.cloudera.org:8080/download_log?c195c2b0-2845-11e9-a3f1-0242ac110002
Not sure about anyone else, but I get a 404 when I click this. This one works for me though: http://dist-test.cloudera.org:8080/diagnose?key=c195c2b0-2845-11e9-a3f1-0242ac110002



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 16 Apr 2019 03:23:16 +0000
Gerrit-HasComments: Yes

[kudu-CR] log block manager: reenable dead container deletion at runtime

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/13021 )

Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................


Removed reviewer Kudu Jenkins with the following votes:

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>

[kudu-CR] log block manager: reenable dead container deletion at runtime

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, helifu, Hao Hao, 

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

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

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

Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................

log_block_manager: reenable dead container deletion at runtime

A quick timeline:
1. The feature was added and its behavior enabled.
2. Occasionally, block_manager-stress-test would SIGSEGV [1]. These failures
   showed up in the flaky test dashboard.
3. So, a feature flag was added in commit 8dc7904. The flag was false by
   default, only set to true for a handful of tests.
4. In commit 3e0fcc9, the flag was set to true in block_manager-stress-test
   so that we could continue to debug the issue.
5. Except that it hasn't surfaced since then; the last
   block_manager-stress-test failure was on 02/03/19.

It's a mystery as to why there haven't been any block_manager-stress-test
failures since #4. Maybe something isn't working quite right with the
feature flag, or maybe the failure was due to something else. Either way,
let's try to enable the feature again, since it's still early in the 1.10
release cycle.

1. http://dist-test.cloudera.org:8080/download_log?key=c195c2b0-2845-11e9-a3f1-0242ac110002

Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
3 files changed, 1 insertion(+), 15 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>

[kudu-CR] log block manager: reenable dead container deletion at runtime

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

Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................

log_block_manager: reenable dead container deletion at runtime

A quick timeline:
1. The feature was added and its behavior enabled.
2. Occasionally, block_manager-stress-test would SIGSEGV [1]. These failures
   showed up in the flaky test dashboard.
3. So, a feature flag was added in commit 8dc7904. The flag was false by
   default, only set to true for a handful of tests.
4. In commit 3e0fcc9, the flag was set to true in block_manager-stress-test
   so that we could continue to debug the issue.
5. Except that it hasn't surfaced since then; the last
   block_manager-stress-test failure was on 02/03/19.

It's a mystery as to why there haven't been any block_manager-stress-test
failures since #4. Maybe something isn't working quite right with the
feature flag, or maybe the failure was due to something else. Either way,
let's try to enable the feature again, since it's still early in the 1.10
release cycle.

1. http://dist-test.cloudera.org:8080/download_log?key=c195c2b0-2845-11e9-a3f1-0242ac110002

Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Reviewed-on: http://gerrit.cloudera.org:8080/13021
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/fs/block_manager-stress-test.cc
M src/kudu/fs/log_block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
3 files changed, 1 insertion(+), 15 deletions(-)

Approvals:
  Andrew Wong: Looks good to me, approved
  Hao Hao: Looks good to me, approved
  Adar Dembo: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: helifu <hz...@corp.netease.com>

[kudu-CR] log block manager: reenable dead container deletion at runtime

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

Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Glad to see this feature moving forward.

http://gerrit.cloudera.org:8080/#/c/13021/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13021/1//COMMIT_MSG@26
PS1, Line 26: 1. http://dist-test.cloudera.org:8080/download_log?c195c2b0-2845-11e9-a3f1-0242ac110002
> Not sure about anyone else, but I get a 404 when I click this. This one wor
404 too.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 16 Apr 2019 12:20:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] log block manager: reenable dead container deletion at runtime

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

Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 16 Apr 2019 19:06:05 +0000
Gerrit-HasComments: No

[kudu-CR] log block manager: reenable dead container deletion at runtime

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

Change subject: log_block_manager: reenable dead container deletion at runtime
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13021/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13021/1//COMMIT_MSG@26
PS1, Line 26: 1. http://dist-test.cloudera.org:8080/download_log?key=c195c2b0-2845-11e9-a3f1-0242ac11
> 404 too.
I messed up the URL. Will fix.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icd8742c150e1447e7838d9f86173536b5726cd40
Gerrit-Change-Number: 13021
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: helifu <hz...@corp.netease.com>
Gerrit-Comment-Date: Tue, 16 Apr 2019 18:54:00 +0000
Gerrit-HasComments: Yes