Skip to content

Commit 70c7d68

Browse files
authored
Add function to allow checking thread type (#4645)
Closes #4613 This change adds a function `threadIsType` function to `Application` that allows a thread to check its type. This is intended to support more detailed assertions than the usual `releaseAssert(threadIsMain())` assertions we're currently using everywhere. The implementation differs a bit from the proposed solution in #4613 as it uses a mapping in `ApplicationImpl` to track thread types, rather than using static variables. I chose this approach because as far as I can tell, it's not possible to assign a thread an id. Given that, `ApplicationImpl` would need to set these variables in its constructor, and the variables would hold meaningless values prior to that. I figure it's safer to ensure that thread types can only be reasoned about after the creation of the threads themselves in `ApplicationImpl`'s constructor. However, if it's important that thread types be reasoned about without an `Application` or `AppConnector`, then I'm open to changing the design. # Checklist - [x] Reviewed the [contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes) document - [x] Rebased on top of master (no merge commits) - [x] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio extension) - [x] Compiles - [x] Ran all tests - [ ] If change impacts performance, include supporting evidence per the [performance document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)
2 parents bf55e27 + d1ae2d7 commit 70c7d68

6 files changed

+44
-2
lines changed

src/main/AppConnector.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,12 @@ AppConnector::checkScheduledAndCache(
140140
return mApp.getOverlayManager().checkScheduledAndCache(msgTracker);
141141
}
142142

143+
bool
144+
AppConnector::threadIsType(Application::ThreadType type) const
145+
{
146+
return mApp.threadIsType(type);
147+
}
148+
143149
SearchableHotArchiveSnapshotConstPtr
144150
AppConnector::copySearchableHotArchiveBucketListSnapshot()
145151
{

src/main/AppConnector.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
#pragma once
22

33
#include "bucket/BucketUtils.h"
4+
#include "main/Application.h"
45
#include "main/Config.h"
56
#include "medida/metrics_registry.h"
67

78
namespace stellar
89
{
9-
class Application;
1010
class OverlayManager;
1111
class LedgerManager;
1212
class Herder;
@@ -57,6 +57,7 @@ class AppConnector
5757
checkScheduledAndCache(std::shared_ptr<CapacityTrackedMessage> msgTracker);
5858
SorobanNetworkConfig const& getSorobanNetworkConfigReadOnly() const;
5959
SorobanNetworkConfig const& getSorobanNetworkConfigForApply() const;
60+
bool threadIsType(Application::ThreadType type) const;
6061

6162
medida::MetricsRegistry& getMetrics() const;
6263
SearchableHotArchiveSnapshotConstPtr

src/main/Application.h

+13
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,16 @@ class Application
164164
APP_NUM_STATE
165165
};
166166

167+
// Types of threads that may be running
168+
enum class ThreadType
169+
{
170+
MAIN,
171+
WORKER,
172+
EVICTION,
173+
OVERLAY,
174+
APPLY
175+
};
176+
167177
virtual ~Application(){};
168178

169179
virtual void initialize(bool createNewDB, bool forceRebuild) = 0;
@@ -330,6 +340,9 @@ class Application
330340
return ret;
331341
}
332342

343+
// Returns true iff the calling thread has the same type as `type`
344+
virtual bool threadIsType(ThreadType type) const = 0;
345+
333346
virtual AppConnector& getAppConnector() = 0;
334347

335348
protected:

src/main/ApplicationImpl.cpp

+14
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,14 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg)
160160
releaseAssert(mConfig.WORKER_THREADS > 0);
161161
releaseAssert(mEvictionIOContext);
162162

163+
mThreadTypes[std::this_thread::get_id()] = ThreadType::MAIN;
164+
163165
// Allocate one thread for Eviction scan
164166
mEvictionThread = std::thread{[this]() {
165167
runCurrentThreadWithMediumPriority();
166168
mEvictionIOContext->run();
167169
}};
170+
mThreadTypes[mEvictionThread->get_id()] = ThreadType::EVICTION;
168171

169172
--t;
170173

@@ -174,19 +177,22 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg)
174177
runCurrentThreadWithLowPriority();
175178
mWorkerIOContext.run();
176179
}};
180+
mThreadTypes[thread.get_id()] = ThreadType::WORKER;
177181
mWorkerThreads.emplace_back(std::move(thread));
178182
}
179183

180184
if (mConfig.BACKGROUND_OVERLAY_PROCESSING)
181185
{
182186
// Keep priority unchanged as overlay processes time-sensitive tasks
183187
mOverlayThread = std::thread{[this]() { mOverlayIOContext->run(); }};
188+
mThreadTypes[mOverlayThread->get_id()] = ThreadType::OVERLAY;
184189
}
185190

186191
if (mConfig.parallelLedgerClose())
187192
{
188193
mLedgerCloseThread =
189194
std::thread{[this]() { mLedgerCloseIOContext->run(); }};
195+
mThreadTypes[mLedgerCloseThread->get_id()] = ThreadType::APPLY;
190196
}
191197
}
192198

@@ -1197,6 +1203,14 @@ ApplicationImpl::getMetrics()
11971203
return *mMetrics;
11981204
}
11991205

1206+
bool
1207+
ApplicationImpl::threadIsType(ThreadType type) const
1208+
{
1209+
auto it = mThreadTypes.find(std::this_thread::get_id());
1210+
releaseAssert(it != mThreadTypes.end());
1211+
return it->second == type;
1212+
}
1213+
12001214
void
12011215
ApplicationImpl::syncOwnMetrics()
12021216
{

src/main/ApplicationImpl.h

+7
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ class ApplicationImpl : public Application
115115
manualClose(std::optional<uint32_t> const& manualLedgerSeq,
116116
std::optional<TimePoint> const& manualCloseTime) override;
117117

118+
bool threadIsType(ThreadType type) const override;
119+
118120
#ifdef BUILD_TESTS
119121
virtual void generateLoad(GeneratedLoadConfig cfg) override;
120122

@@ -220,6 +222,11 @@ class ApplicationImpl : public Application
220222
// thread for eviction scans.
221223
std::optional<std::thread> mEvictionThread;
222224

225+
// NOTE: It is important that this map not be updated outside of the
226+
// constructor. `unordered_map` is safe for multiple threads to read from,
227+
// so long as there are no concurrent writers.
228+
std::unordered_map<std::thread::id, Application::ThreadType> mThreadTypes;
229+
223230
asio::signal_set mStopSignals;
224231

225232
bool mStarted;

src/overlay/Peer.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,8 @@ void
460460
Peer::maybeExecuteInBackground(std::string const& jobName,
461461
std::function<void(std::shared_ptr<Peer>)> f)
462462
{
463-
if (useBackgroundThread() && threadIsMain())
463+
if (useBackgroundThread() &&
464+
!mAppConnector.threadIsType(Application::ThreadType::OVERLAY))
464465
{
465466
mAppConnector.postOnOverlayThread(
466467
[self = shared_from_this(), f]() { f(self); }, jobName);

0 commit comments

Comments
 (0)