Feature/precommit batcher #422
Conversation
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Codecov Report
@@ Coverage Diff @@
## master #422 +/- ##
==========================================
+ Coverage 44.73% 44.86% +0.12%
==========================================
Files 603 607 +4
Lines 26374 26458 +84
Branches 14219 14263 +44
==========================================
+ Hits 11799 11870 +71
- Misses 11263 11268 +5
- Partials 3312 3320 +8
Continue to review full report at Codecov.
|
| batcher->cutoffStart = std::chrono::system_clock::now(); | ||
| batcher->closestCutoff = maxWait; | ||
| batcher->handle_ = scheduler->schedule(maxWait, [=]() { | ||
| auto maybe_send = batcher->sendBatch(); |
There was a problem hiding this comment.
Signature here
using Callback = std::function<void()>; only void function is allowed. maybe_send.has_error() would be appropriate
|
Do not forget to integrate this part to |
…cosmetic changes Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: elestrias <rus8-2002@mail.ru>
| } | ||
|
|
||
| void PreCommitBatcherImpl::forceSend() { | ||
| auto maybe_result = sendBatch(); |
There was a problem hiding this comment.
Check maybe_result.has_error()
There was a problem hiding this comment.
I decided not to check the error, just to add the outcome::result object to provided callbacks
There was a problem hiding this comment.
And then handle error in caller proccess
There was a problem hiding this comment.
I'm not sure that it is a good idea.
Callbacks must consider that parameter could have an error.
@ortyomka what do you think?
Also I don't see any test with non-void callback
There was a problem hiding this comment.
This like subscibtion, this error is important for those who signed up to send the batch.
But we can duplicate the error to the one who called Force.
I think this the best option
There was a problem hiding this comment.
the caller of sendForce can check callback if he needs to it, otherwise I cant imagine a situation of usage this functional, maybe logs can be used just to inform?
| batcher->logger_ = common::createLogger("batcher"); | ||
| batcher->logger_->info("Bather have been started"); | ||
| batcher->handle_ = scheduler->schedule(max_wait, [=]() { | ||
| auto maybe_result = batcher->sendBatch(); |
There was a problem hiding this comment.
Check maybe_result.has_error()
Signed-off-by: elestrias <rus8-2002@mail.ru>
…ecks Signed-off-by: elestrias <rus8-2002@mail.ru>
Signed-off-by: Mikhail Tagirov <dev.mikhail.tagirov@gmail.com>
Signed-off-by: elestrias <rus8-2002@mail.ru>
…t/cpp-filecoin into feature/PrecommitBatcher
Description of the Change
A new model of a PreCommit messages sending via batch. The mechanism a collection and a storage of all the recieved messages until the deadline, when all the collected data will be sent as one message.
Benefits
This feature will reduce the growth of transaction fees on the network as the general amount of transactions will be reduced.
Sending a several messages through one transaction will reduce spendings of miner's resourses
Possible Drawbacks
Precommit sending system are generaly changed, so the thrusted previous one mechanism should not be used anymore in general cases.
Usage Examples or Tests [optional]
With usage of batching, miner wont send its Precommit messages ocasionally as they appears, but several Precommits in one transaction.
Alternate Designs [optional]
The system can be realised via hook, but in the future perspective it can take too much resourses
to support more than two threads for batching