[#2152] neofs-node: Update container list cache properly #2176
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2176 +/- ##
==========================================
- Coverage 30.75% 30.74% -0.01%
==========================================
Files 382 382
Lines 28230 28231 +1
==========================================
Hits 8681 8681
- Misses 18818 18819 +1
Partials 731 731
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
cmd/neofs-node/cache.go
Outdated
| @@ -312,41 +312,41 @@ func (s *ttlContainerLister) List(id *user.ID) ([]cid.ID, error) { | |||
| // All described race cases aren't critical since cache values expire anyway, we just try | |||
| // to increase cache actuality w/o huge overhead on synchronization. | |||
| func (s *ttlContainerLister) update(owner user.ID, cnr cid.ID, add bool) { | |||
| strOwner := owner.EncodeToString() | |||
| for _, key := range []string{"", owner.EncodeToString()} { | |||
There was a problem hiding this comment.
"" for "owner-less" containers? Why in this order then? I'd expect a check for direct owner hit first and a fallback then.
There was a problem hiding this comment.
Actually, we need both. continue inside the cycle is just for the case when we do not cache the result.
There was a problem hiding this comment.
Can "" case grow a lot in some cases? What if a container is cached as owner-less, but then is added with an owner? Something's fishy here.
There was a problem hiding this comment.
Nothing bad happens -- both lists are completely independent.
I have removed caching for the full-list because on close inspection we do not really use it that often.
…Writer` Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
`Get` needs write mutex and makes our item move to top. Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
We rarely need to list all containers: as one example we need it for tree service synchronization once per epoch. Given that cache TTL has the order of block time it makes no sense to cache the list of all containers. Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>
| @@ -314,16 +315,21 @@ func (s *ttlContainerLister) List(id *user.ID) ([]cid.ID, error) { | |||
| func (s *ttlContainerLister) update(owner user.ID, cnr cid.ID, add bool) { | |||
| strOwner := owner.EncodeToString() | |||
|
|
|||
| val, ok := (*ttlNetCache)(s).cache.Get(strOwner) | |||
| val, ok := s.inner.cache.Peek(strOwner) | |||
There was a problem hiding this comment.
what does Get -> Peek change solve?
There was a problem hiding this comment.
Peek is faster (read-only) and does not update the position of an item in LRU cache -- which is what we do here, we don't use anything, just passively update.
Close #2152 .
Close #2155. (?) @acid-ant
Close #2052. (?)
Update the list of all containers if needed.
Signed-off-by: Evgenii Stratonikov e.stratonikov@yadro.com