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/03/19 03:44:57 UTC
[kudu-CR] [threadpool] fix race in ThreadPoolToken::Submit()
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17205
Change subject: [threadpool] fix race in ThreadPoolToken::Submit()
......................................................................
[threadpool] fix race in ThreadPoolToken::Submit()
This patch fixes a race between calling ThreadPoolToken::Submit()
and destructing the token concurrently from another thread.
The patch moves the update of the thread pool token's queue length
metric under the lock used in the ThreadPool::DoSubmit() method.
The motivation for this patch was seeing the following TSAN report
while running Params/ScanYourWritesParamTest.Test/1:
WARNING: ThreadSanitizer: data race (pid=18290)
Write of size 8 at 0x7b2c0002b0e8 by main thread:
#0 operator delete(void*)
#1 std::__1::default_delete<kudu::ThreadPoolToken>::operator()(kudu::ThreadPoolToken*)
#2 std::__1::unique_ptr<kudu::ThreadPoolToken, std::__1::default_delete<kudu::ThreadPoolToken> >::reset(kudu::ThreadPoolToken*)
#3 std::__1::unique_ptr<kudu::ThreadPoolToken, std::__1::default_delete<kudu::ThreadPoolToken> >::~unique_ptr()
#4 kudu::consensus::RaftConsensus::~RaftConsensus() src/kudu/consensus/raft_consensus.cc:210:1
#5 ...
#6 std::__1::__shared_count::__release_shared()
#7 std::__1::__shared_weak_count::__release_shared()
#8 std::__1::shared_ptr<kudu::consensus::RaftConsensus>::~shared_ptr()
#9 kudu::tablet::TabletReplica::~TabletReplica() src/kudu/tablet/tablet_replica.cc:195:1
.....
Previous read of size 8 at 0x7b2c0002b0e8 by thread T125:
#0 scoped_refptr<kudu::Histogram>::operator kudu::Histogram* scoped_refptr<kudu::Histogram>::*() const
#1 kudu::ThreadPool::DoSubmit(std::__1::function<void ()>, kudu::ThreadPoolToken*) src/kudu/util/threadpool.cc:523:7
#2 kudu::ThreadPoolToken::Submit(std::__1::function<void ()>) src/kudu/util/threadpool.cc:124:17
#3 kudu::consensus::Peer::SignalRequest(bool) src/kudu/consensus/consensus_peers.cc:188:28
#4 kudu::consensus::Peer::Init()::$_0::operator()() src/kudu/consensus/consensus_peers.cc:161:14
#5 ...
#6 ...
#7 ...
#8 ...
#9 ...
#10 ...
#11 kudu::rpc::PeriodicTimer::Callback(long)
.....
Change-Id: I2b17e4b2b634624fbc51e8ee05749a56f6609f62
---
M src/kudu/util/threadpool.cc
1 file changed, 11 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/05/17205/1
--
To view, visit http://gerrit.cloudera.org:8080/17205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2b17e4b2b634624fbc51e8ee05749a56f6609f62
Gerrit-Change-Number: 17205
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [threadpool] fix race in ThreadPoolToken::Submit()
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17205 )
Change subject: [threadpool] fix race in ThreadPoolToken::Submit()
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b17e4b2b634624fbc51e8ee05749a56f6609f62
Gerrit-Change-Number: 17205
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 19 Mar 2021 14:38:07 +0000
Gerrit-HasComments: No
[kudu-CR] [threadpool] fix race in ThreadPoolToken::Submit()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17205 )
Change subject: [threadpool] fix race in ThreadPoolToken::Submit()
......................................................................
Patch Set 1: Verified+1
unrelated test failure in TabletHistoryGcITest.TestUndoDeltaBlockGc
--
To view, visit http://gerrit.cloudera.org:8080/17205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b17e4b2b634624fbc51e8ee05749a56f6609f62
Gerrit-Change-Number: 17205
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: Fri, 19 Mar 2021 04:55:27 +0000
Gerrit-HasComments: No
[kudu-CR] [threadpool] fix race in ThreadPoolToken::Submit()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17205 )
Change subject: [threadpool] fix race in ThreadPoolToken::Submit()
......................................................................
[threadpool] fix race in ThreadPoolToken::Submit()
This patch fixes a race between calling ThreadPoolToken::Submit()
and destructing the token concurrently from another thread.
The patch moves the update of the thread pool token's queue length
metric under the lock used in the ThreadPool::DoSubmit() method.
The motivation for this patch was seeing the following TSAN report
while running Params/ScanYourWritesParamTest.Test/1:
WARNING: ThreadSanitizer: data race (pid=18290)
Write of size 8 at 0x7b2c0002b0e8 by main thread:
#0 operator delete(void*)
#1 std::__1::default_delete<kudu::ThreadPoolToken>::operator()(kudu::ThreadPoolToken*)
#2 std::__1::unique_ptr<kudu::ThreadPoolToken, std::__1::default_delete<kudu::ThreadPoolToken> >::reset(kudu::ThreadPoolToken*)
#3 std::__1::unique_ptr<kudu::ThreadPoolToken, std::__1::default_delete<kudu::ThreadPoolToken> >::~unique_ptr()
#4 kudu::consensus::RaftConsensus::~RaftConsensus() src/kudu/consensus/raft_consensus.cc:210:1
#5 ...
#6 std::__1::__shared_count::__release_shared()
#7 std::__1::__shared_weak_count::__release_shared()
#8 std::__1::shared_ptr<kudu::consensus::RaftConsensus>::~shared_ptr()
#9 kudu::tablet::TabletReplica::~TabletReplica() src/kudu/tablet/tablet_replica.cc:195:1
.....
Previous read of size 8 at 0x7b2c0002b0e8 by thread T125:
#0 scoped_refptr<kudu::Histogram>::operator kudu::Histogram* scoped_refptr<kudu::Histogram>::*() const
#1 kudu::ThreadPool::DoSubmit(std::__1::function<void ()>, kudu::ThreadPoolToken*) src/kudu/util/threadpool.cc:523:7
#2 kudu::ThreadPoolToken::Submit(std::__1::function<void ()>) src/kudu/util/threadpool.cc:124:17
#3 kudu::consensus::Peer::SignalRequest(bool) src/kudu/consensus/consensus_peers.cc:188:28
#4 kudu::consensus::Peer::Init()::$_0::operator()() src/kudu/consensus/consensus_peers.cc:161:14
#5 ...
#6 ...
#7 ...
#8 ...
#9 ...
#10 ...
#11 kudu::rpc::PeriodicTimer::Callback(long)
.....
Change-Id: I2b17e4b2b634624fbc51e8ee05749a56f6609f62
Reviewed-on: http://gerrit.cloudera.org:8080/17205
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/util/threadpool.cc
1 file changed, 11 insertions(+), 3 deletions(-)
Approvals:
Alexey Serbin: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/17205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2b17e4b2b634624fbc51e8ee05749a56f6609f62
Gerrit-Change-Number: 17205
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [threadpool] fix race in ThreadPoolToken::Submit()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [threadpool] fix race in ThreadPoolToken::Submit()
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/17205
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I2b17e4b2b634624fbc51e8ee05749a56f6609f62
Gerrit-Change-Number: 17205
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)