You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Wang Xixu (Code Review)" <ge...@cloudera.org> on 2022/11/04 10:54:33 UTC

[kudu-CR] [KUDU-3419][TabletManager] Fix the stuck of tablet server when starting

Wang Xixu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19203


Change subject: [KUDU-3419][TabletManager] Fix the stuck of tablet server when starting
......................................................................

[KUDU-3419][TabletManager] Fix the stuck of tablet server when starting

If the permission of tablet metadata file is denied when tablet server starting,
tablet server will get stuck and can not exit automatically. That is because
if the permission is denied, the object of TabletServer will be deconstructed,
it will deconstruct it's thread pool:txn_status_manager_pool_, but this thread
pool contains a task: TxnStalenessTrackerTask, the task is still running.
There are not change to finish this task. Therefore tablet server will get stuck.
See the detail at #KUDU-3419

Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
---
M src/kudu/tserver/ts_tablet_manager.cc
1 file changed, 25 insertions(+), 10 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 1
Gerrit-Owner: Wang Xixu <14...@qq.com>

[kudu-CR] [KUDU-3419][TabletManager] Fix the stuck of tablet server when starting

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.

Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19203 )

Change subject: [KUDU-3419][TabletManager] Fix the stuck of tablet server when starting
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/19203/5/src/kudu/tserver/ts_tablet_manager.h
File src/kudu/tserver/ts_tablet_manager.h:

http://gerrit.cloudera.org:8080/#/c/19203/5/src/kudu/tserver/ts_tablet_manager.h@200
PS5, Line 200:       std::vector<scoped_refptr<tablet::TabletReplica>>* replicas) const override;
Leave a blank line between the function and the next function (comments)


http://gerrit.cloudera.org:8080/#/c/19203/4/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19203/4/src/kudu/tserver/ts_tablet_manager.cc@1586
PS4, Line 1586: void TSTabletManager::GetLocalTabletReplicas(
              :             vector<scoped_refptr<TabletReplica>>* replicas) const {
              :   shared_lock<RWMutex> l(lock_);
              :   AppendValuesFromMap(tablet_map_, replicas);
              : }
              : 
              : void TSTabletManager::GetTabletReplicas(vector<scoped_refptr<TabletReplica>>* replicas) const {
              :   shared_lock<RWMutex> l(lock_);
              :   AppendValuesFromMap(tablet_map_, replicas);
              : }
              : 
> No difference. GetTabletReplicas is virtual, so it can be not used in decon
What happend if you use it?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 5
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 10:52:33 +0000
Gerrit-HasComments: Yes

[kudu-CR] [KUDU-3419] Fix the stuck of tablet server when starting

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19203 )

Change subject: [KUDU-3419] Fix the stuck of tablet server when starting
......................................................................


Patch Set 6:

(4 comments)

Any chance to add a test for this to catch regressions in the future?

http://gerrit.cloudera.org:8080/#/c/19203/4//COMMIT_MSG
Commit Message:

PS4: 
> Could you reformat the commit message a bit?  Please follow the guide at ht
Missed this?


http://gerrit.cloudera.org:8080/#/c/19203/4//COMMIT_MSG@12
PS4, Line 12: its 
> If tablet server does not finished initiation, UnregisterAllServices() will
Right, but the idea was to update that code appropriately.

OK, if you think it's not worth it, the current approach is good to fix the issue.


http://gerrit.cloudera.org:8080/#/c/19203/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19203/6//COMMIT_MSG@8
PS6, Line 8: 
           : If the permission of tablet metadata file is denied when tablet server starting,
           : tablet server will get stuck and can not exit automatically. That is because
           : if the permission is denied, the object of TabletServer will be deconstructed,
           : it will deconstruct its thread pool:txn_status_manager_pool_, but this thread
           : pool contains a task: TxnStalenessTrackerTask, the task is still running.
           : There are no code to shutdown the task in that case. Therefore tablet server
           : will get stuck.
Could you reformat the commit message a bit?  Please follow the guide at https://git-scm.com/book/en/v2/Distributed-Git-Contributing-to-a-Project#_commit_guidelines that can be found at the https://kudu.apache.org/docs/contributing.html#_submitting_patches page.

In essence, strive to keep the lines in the description to be at most 72 characters long.


http://gerrit.cloudera.org:8080/#/c/19203/6/src/kudu/tserver/ts_tablet_manager.h
File src/kudu/tserver/ts_tablet_manager.h:

http://gerrit.cloudera.org:8080/#/c/19203/6/src/kudu/tserver/ts_tablet_manager.h@293
PS6, Line 293:     
nit: there should be 4 spaces indent here, see the code style guide https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_Definitions



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 15:55:00 +0000
Gerrit-HasComments: Yes

[kudu-CR] [KUDU-3419] Fix the stuck of tablet server when starting

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.

Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/19203 )

Change subject: [KUDU-3419] Fix the stuck of tablet server when starting
......................................................................


Patch Set 6:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/19203/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19203/4//COMMIT_MSG@7
PS4, Line 7: [KUDU-3419] Fix the stuck 
> nit: use either 'KUDU-3419: ...' or '[TabletManager] ...', but not both
Done


http://gerrit.cloudera.org:8080/#/c/19203/4//COMMIT_MSG@12
PS4, Line 12: its 
> its
If tablet server does not finished initiation, UnregisterAllServices() will failed, because messenger_ is nlullptr


http://gerrit.cloudera.org:8080/#/c/19203/4//COMMIT_MSG@14
PS4, Line 14: There are no code to shutdown the task i
> nit: There was no code to shutdown the task in that case.
Done


http://gerrit.cloudera.org:8080/#/c/19203/4//COMMIT_MSG@15
PS4, Line 15: will get stuck.
> nit: See KUDU-3419 for details.
Done


http://gerrit.cloudera.org:8080/#/c/19203/5/src/kudu/tserver/ts_tablet_manager.h
File src/kudu/tserver/ts_tablet_manager.h:

http://gerrit.cloudera.org:8080/#/c/19203/5/src/kudu/tserver/ts_tablet_manager.h@200
PS5, Line 200:   // next round of master heartbeats.
> Leave a blank line between the function and the next function (comments)
Done


http://gerrit.cloudera.org:8080/#/c/19203/4/src/kudu/tserver/ts_tablet_manager.h
File src/kudu/tserver/ts_tablet_manager.h:

http://gerrit.cloudera.org:8080/#/c/19203/4/src/kudu/tserver/ts_tablet_manager.h@196
PS4, Line 196:   void GetTabletReplicas(
             :       std::vector<scoped_refptr<tablet::TabletReplica>>* replicas) const o
> Make this method private and call it in the implementation of the GetTablet
Done


http://gerrit.cloudera.org:8080/#/c/19203/4/src/kudu/tserver/ts_tablet_manager.h@200
PS4, Line 200: 
> nit: remove -- contemporary C++ compilers don't require this extra space
Done


http://gerrit.cloudera.org:8080/#/c/19203/4/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:

http://gerrit.cloudera.org:8080/#/c/19203/4/src/kudu/tserver/ts_tablet_manager.cc@553
PS4, Line 553: LOG_AND_RETURN(ERROR, first_error);
> I guess this should have become
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 11:52:59 +0000
Gerrit-HasComments: Yes

[kudu-CR] [KUDU-3419] Fix the stuck of starting tablet server

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.

Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19203 )

Change subject: [KUDU-3419] Fix the stuck of starting tablet server
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Nov 2022 02:20:41 +0000
Gerrit-HasComments: No

[kudu-CR] [KUDU-3419] Fix the stuck of starting tablet server

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19203 )

Change subject: [KUDU-3419] Fix the stuck of starting tablet server
......................................................................


Patch Set 8: Verified+1

unrelated test failures


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Nov 2022 18:04:24 +0000
Gerrit-HasComments: No

[kudu-CR] [KUDU-3419] Fix the stuck of starting tablet server

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has removed a vote on this change.

Change subject: [KUDU-3419] Fix the stuck of starting tablet server
......................................................................


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 8
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [KUDU-3419] Fix the stuck of tablet server when starting

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.

Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19203 )

Change subject: [KUDU-3419] Fix the stuck of tablet server when starting
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Nov 2022 12:02:32 +0000
Gerrit-HasComments: No

[kudu-CR] [KUDU-3419] Fix the stuck of tablet server when starting

Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.

Hello Tidy Bot, Alexey Serbin, Yingchun Lai, Kudu Jenkins, KeDeng, 

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

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

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

Change subject: [KUDU-3419] Fix the stuck of tablet server when starting
......................................................................

[KUDU-3419] Fix the stuck of tablet server when starting

If the permission of tablet metadata file is denied when tablet server starting,
tablet server will get stuck and can not exit automatically. That is because
if the permission is denied, the object of TabletServer will be deconstructed,
it will deconstruct its thread pool:txn_status_manager_pool_, but this thread
pool contains a task: TxnStalenessTrackerTask, the task is still running.
There are no code to shutdown the task in that case. Therefore tablet server
will get stuck.

See #KUDU-3419 for detail

Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
---
M src/kudu/tserver/ts_tablet_manager.cc
M src/kudu/tserver/ts_tablet_manager.h
2 files changed, 45 insertions(+), 16 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8c9a4f4158fcb0a36499345e00ee72c65f5fefe0
Gerrit-Change-Number: 19203
Gerrit-PatchSet: 6
Gerrit-Owner: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>