runtime-api: do not cache None SessionInfo #4706
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
ordian
added
A2-insubstantial
ordian
force-pushed
the
ao-approval-voting-look-back
branch
from
42b27b0 to
7a195db
Compare
ordian
force-pushed
the
ao-runtime-cache-session-info-fix
branch
from
65a5d4d to
8b36ba1
Compare
ordian
force-pushed
the
ao-approval-voting-look-back
branch
from
7a195db to
c4b4d84
Compare
ordian
force-pushed
the
ao-runtime-cache-session-info-fix
branch
from
8b36ba1 to
143923d
Compare
|
Nice catch |
|
bot merge |
paritytech-processbot
bot
deleted the
ao-runtime-cache-session-info-fix
branch
| self.session_info.insert(key, ResidentSizeOf(value)); | ||
| // only cache Some(SessionInfo) | ||
| if value.is_some() { | ||
| self.session_info.insert(key, ResidentSizeOf(value)); |
There was a problem hiding this comment.
Why do we even cache Option<SessionInfo> if we only insert Some?
There was a problem hiding this comment.
Because of the macro it's used in. I've tried to change that, but couldn't figure out how to make it compile w/o changing the macro in 5 minutes, so gave up on that.
There was a problem hiding this comment.
That should really not be the way to go. If you have problems with this things, you should ask and not release a half backed solution.
Any idea why were we request "invalid session info" aka with a not yet running session index?
There was a problem hiding this comment.
If you have problems with this things, you should ask and not release a half backed solution.
I don't view it as a half baked solution. It's possible to refactor the macro code to be more generic, but I'm not sure what would be the benefit of that. It will likely be less readable and this is only "nice to have". Besides that the fix is minimal and easy to backport if needed.
Any idea why were we request "invalid session info" aka with a not yet running session index?
Not sure why it happened on Kusama. Note that this was implemented as a response to Rococo issue when we failed to scale decode wrong SessionInfo definition, so it returned None.
* approval-voting: add more logs * approval-voting: query finalized block on startup and increase look back * runtime-api: do not cache None SessionInfo
(see the previous PR description)