-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor: thread pool using #1986
Conversation
outcome::result<WarpSyncProof> getProof( | ||
const primitives::BlockHash &after_hash) const; | ||
|
||
void warp(const primitives::BlockInfo &block); | ||
|
||
private: | ||
outcome::result<void> cacheMore(primitives::BlockNumber finalized); | ||
outcome::result<void> start(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outcome::result<void> start(); | |
outcome::result<void> startOutcome(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
main_pool_handler, | ||
worker_pool_handler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main_pool_handler, | |
worker_pool_handler); | |
TestThreadPool{io_}, | |
io_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wont work 'cause arguments have a strict type, and TestThreadPool is not derived of them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works in that branch (also strong type, with constructor(TestThreadPool)
)
: is_active_{false}, ioc_{std::move(io_context)} {} | ||
~PoolHandler() = default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be constructed as "active",
or will lose callbacks before AppStateManager
calls start()
: is_active_{false}, ioc_{std::move(io_context)} {} | |
~PoolHandler() = default; | |
: is_active_{true}, ioc_{std::move(io_context)} {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making it active is responsibility of using class. To avoid callback loosing, class-owner should start pool handler at 'prepare' phase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we fix PoolHandler { is_active_ = true }
constructor,
or each class constructor Foo(PoolHandler pool_handler) { pool_handler.start() }
?
app_state_manager_->takeControl(*statement_fetching_protocol_.get()); | ||
|
||
app_state_manager_->takeControl(*send_dispute_protocol_.get()); | ||
main_pool_handler_->execute([wp = weak_from_this()] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing setProtocolHandler
main_pool_handler_->execute([wp = weak_from_this()] { | |
REINVOKE(main_pool_handler_) |
if (enabled_) { | ||
message_pool_ = std::make_shared<MessagePool>( | ||
kTelemetryMessageMaxLengthBytes, kTelemetryMessagePoolSize); | ||
app_state_manager_->takeControl(*this); | ||
} else { | ||
SL_INFO(log_, "Telemetry disabled"); | ||
} | ||
} | ||
|
||
bool TelemetryServiceImpl::prepare() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bool TelemetryServiceImpl::prepare() { | |
bool TelemetryServiceImpl::prepare() { | |
REINVOKE(io_context_) |
return true; | ||
} | ||
|
||
bool TelemetryServiceImpl::start() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return true; | |
} | |
bool TelemetryServiceImpl::start() { |
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
…hread pool) Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
a1dd273
to
629e7b3
Compare
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Signed-off-by: Dmitriy Khaustov aka xDimon <[email protected]>
Referenced issues
Description of the Change
Benefits
Possible Drawbacks
Usage Examples or Tests
Alternate Designs