-
Notifications
You must be signed in to change notification settings - Fork 113
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
Create State
implementation with registering all of the specific ReadableKVStates
#9700
Conversation
State
implementation with registering all of the specific ReadableKVStates
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9700 +/- ##
============================================
+ Coverage 92.23% 92.25% +0.02%
- Complexity 7628 7739 +111
============================================
Files 937 945 +8
Lines 32119 32343 +224
Branches 4071 4105 +34
============================================
+ Hits 29624 29838 +214
Misses 1542 1542
- Partials 953 963 +10 ☔ View full report in Codecov by Sentry. |
b20e20b
to
ce8ef43
Compare
hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/state/utils/MapWritableKVState.java
Outdated
Show resolved
Hide resolved
ce8ef43
to
7b1d58d
Compare
b59a4ff
to
cd96150
Compare
hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/state/utils/ListReadableQueueState.java
Outdated
Show resolved
Hide resolved
hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/state/MirrorNodeState.java
Show resolved
Hide resolved
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
f3d60cf
to
fa57b07
Compare
...a-mirror-web3/src/main/java/com/hedera/mirror/web3/state/utils/AbstractMapReadableState.java
Outdated
Show resolved
Hide resolved
hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/state/utils/ListWritableQueueState.java
Outdated
Show resolved
Hide resolved
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.
At first glance the PR looks reasonable and have all of the needed state core components. The state should be utilized via integration tests (added in separate PR/PRs) to validate the correctness and consistency of the state. A lot of pitfalls or little issues can be missed without running actual transactions against this state. So I think this PR can be merged in this form and if it needs some touch ups - they can be made in separate PRs.
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Signed-off-by: Bilyana Gospodinova <bilyana.gospodinova14@gmail.com>
Quality Gate passedIssues Measures |
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.
Mostly just questions
* {@link Map}. Test code has the option of creating an instance disregarding the backing map, or by | ||
* supplying the backing map to use. This latter option is useful if you want to use Mockito to spy |
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.
How would one disregard the backing map? It's required and will throw a NPE if not supplied
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 seems that code was updated but the comment was not. I will update it, thanks.
hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/state/core/MapWritableStates.java
Show resolved
Hide resolved
hedera-mirror-web3/src/main/java/com/hedera/mirror/web3/state/MirrorNodeState.java
Show resolved
Hide resolved
data.put(stateName, withAnyRegisteredListeners(serviceName, stateName, ref)); | ||
} | ||
} | ||
return new MapWritableStates(data, () -> readableStates.remove(serviceName)); |
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.
Will this cause a commit to flush the readable state and cause it to get re-created for every request?
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, it will. I tested it without the runnable and it seems to work fine for 1 request. This was added in the example code probably for a reason. I will remove it for now and we will investigate the behaviour on parallel requests later.
Description:
This PR adds a
MirrorNodeState
class that implements theState
interface. The class registers all of needed readable and writable states. Each service (TokenService
,ContractService
orFileService
) needs a different set of states in order to function properly. The readable states are initialised only once and are singleton instances and will be used by all of the threads. They are the implementations of all of theReadableKVState
s. For the writable states we need only 1 facade implementation since no actual changes will be persisted from the memory. The writable state uses a readable state for the read operations and the modifications are kept in memory and discarded at the end of the transaction execution.Fixes #9259