Fixes #17172: Concurrent access to node info cache cause exception to be thrown when accessing cache content #2910
Conversation
| @@ -413,7 +413,10 @@ trait NodeInfoServiceCached extends NodeInfoService with NamedZioLogger with Cac | |||
| } | |||
| } else { | |||
| logPure.debug(s"NodeInfo cache is up to date, ${nodeCache.map(c => s"last modification time: '${c.lastModTime}' for: '${c.lastModEntryCSN.mkString("','")}'").getOrElse("")}") *> | |||
| nodeCache.get.nodeInfos.succeed //get is ok because in a synchronized block with a test on isEmpty | |||
| (nodeCache match { | |||
There was a problem hiding this comment.
could you correct the indentation here ?
There was a problem hiding this comment.
indentation was not correct, this is a parameter of the first line
logPure.debug (xxxx) *> (nodeCache match ....)
if really find it easier to understand that the second line depends on the first one with one more indent level.
When aligned on the same line it took me some time to understand why it did not compile without parenthesis, because first line is too long to see *> on screen
There was a problem hiding this comment.
*> does not means it's a parameter, it's a sequence (like with ;).
You can put *> at the begining to make it more visible.
You may be force to use parenthesis for the else to make scalac happy.
The parenthesis around nodeCache match {} should not be necessary though.
There was a problem hiding this comment.
no without parenthesis it considers that
logPure.debug(s"NodeInfo cache is up to date, ${nodeCache.map(c => s"last modification time: '${c.lastModTime}' for: '${c.lastModEntryCSN.mkString("','")}'").getOrElse("")}") *> nodeCache
is the variable to match and not just nodeCache
it does not work to put it before the second line it does not compile (*> cannot be applied )
anyway i'll go back to old indentation
|
Commit modified |
VinceMacBuche
force-pushed
the
bug_17172/error_when_updateing_dynamic_group
branch
from
edef48b
to
d2cce87
Compare
|
PR updated with a new commit |
|
This PR is not mergeable to upper versions. |
|
OK, squash merging this PR |
… be thrown when accessing cache content
fanf
force-pushed
the
bug_17172/error_when_updateing_dynamic_group
branch
from
527df7b
to
1a43f6d
Compare
https://issues.rudder.io/issues/17172