-
Notifications
You must be signed in to change notification settings - Fork 366
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
INDY-2200: Use production req handlers in simulation tests #1294
Conversation
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
@@ -23,10 +23,10 @@ def readNode(self, pos): | |||
return self._nodes[pos - 1] | |||
|
|||
def readLeafs(self, startpos, endpos): | |||
return (n for n in self._leafs[startpos - 1:endpos - 1]) | |||
return [n for n in self._leafs[startpos - 1:endpos]] |
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.
Won't the original code (generator) be more efficient for huge data?
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.
When I was trying to debug failing tests I noticed that database-backed implementation returns a list, so I returned list here as well so implementations match as close as possible. If some code tries to iterate twice or use indexes then it will fail with generators. As for resource efficiency - with this storage we already have all leaves in memory anyways.
plenum/server/ledgers_bootstrap.py
Outdated
# functions by ledger instead of by initialization stage | ||
# - create request managers and expose as properties instead of passing as parameters, | ||
# just like bls bft | ||
# - if we introduce some entity (named Executor, Application, BusinessLogic or whatever) |
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.
I think that WriteRequestManager
plays the role of executor now. It already contains DatabseManager
.
As for ReadRequestManager
and ActionRequestManager
, I think they don't belong to Executor
interface. This is separate entities which are independent of both consus and transaction execution logic (they can and should go into a separate service/process).
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.
Agree with point on moving them into separate processes. Just a some of comments on that:
- if they are in different processes then they will need to open databases separately. Updates across different databases are not atomic, so some form of synchronization might be needed.
- even though they are in different processes they access same databases (and there are quite a lot of them), so it is still desirable to have common code at least for db initialization
- thinking about observers (running on a separate node) - while they serve only read requests they will most likely require WriteReqManager as well in order to build all those cache databases, and probably state as well
plenum/test/consensus/helper.py
Outdated
|
||
class TestLedgersBootstrap(LedgersBootstrap): | ||
def create_bls_bft(self): | ||
return FakeSomething() |
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 it be implementation of BlsBft interface?
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.
Yes, this is planned after all other things start working :)
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
43e8521
to
f58ab1f
Compare
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
f58ab1f
to
900a0fd
Compare
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
Signed-off-by: Sergey Khoroshavin <sergey.khoroshavin@dsr-corporation.com>
No description provided.