Invalidate container cache on PutSuccess event #272
Labels
No labels
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: TrueCloudLab/frostfs-node#272
Loading…
Reference in a new issue
No description provided.
Delete branch "dstepanov-yadro/frostfs-node:fix/container_not_found"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
For example: frostfs-cli creates container and makes polling GetContainer requests. These requests go through container cache, so not found error stores in container cache. So container cache can contain not found error when PutSuccess event received.
Closes #231
Could you mention
Close #231
in the PR message?@ -135,2 +134,4 @@
cnr, err := cnrSrc.Get(ev.ID)
if err == nil {
cachedContainerLister.update(cnr.Value.Owner(), ev.ID, true)
cachedContainerStorage.set(ev.ID, cnr, nil)
I think we have a race-condition here, can this
set
happen before anotherNotFound
result is in the process of being saved in cache?Actually we have such race condition in TTL cache.
Added lock
There are two PR's, so i close issue manualy
I believe only master PR affects issue state.
fb0295c36b
todf06fa862e
df06fa862e
toa600a5f4b4
@ -18,0 +36,4 @@
l.mtx.Lock()
if locker, found := l.lockers[key]; found {
locker.waiters++
Can we do this without
locker
mtx?l.mtx.Lock()
protectsCan we mention this in the comment to
mtx
orwaiters
field?hm, there are 8 lines of code only.
comment added
It's for the kids!
@ -18,0 +56,4 @@
}
func (l *keyLock[K]) Unlock(key K) {
l.mtx.Lock()
I find it difficult to read and verify the absense of deadlocks: we have multiple mutexes and we call some
Lock
inUnlock
, so the order ofdefer + Unlock()
could matter in some cases.Sorry, i do not know how to make this code more understandable. It seems like there are 10 lines of code and it should be simple. Therefore, I will be glad if you suggest options.
@ -54,6 +115,9 @@ func newNetworkTTLCache[K comparable, V any](sz int, ttl time.Duration, netRdr n
//
// returned value should not be modified.
func (c *ttlNetCache[K, V]) get(key K) (V, error) {
c.keyLocker.Lock(key)
Do we need to block even if the valid item is in cache?
Fixed with two checks
a600a5f4b4
to38974c4c94
38974c4c94
to35214798d7
35214798d7
to238cd3bfd9
@ -33,0 +74,4 @@
if locker.waiters == 1 {
delete(l.lockers, key)
}
locker.waiters--
Do this in the
else
branch? Or is it a deliberate decision? Don't mind either option.It's deliberate decision.
else
is redundant.238cd3bfd9
to90fff809a5
90fff809a5
to943819537a
943819537a
to04be9415d9