Subject delete markers on purges and removes for filestore #6428
Conversation
server/filestore.go
Outdated
| @@ -4491,7 +4495,7 @@ func (fs *fileStore) removeMsg(seq uint64, secure, viaLimits, needFSLock bool) ( | |||
|
|
|||
| // If we are tracking multiple subjects here make sure we update that accounting. | |||
| mb.removeSeqPerSubject(sm.subj, seq) | |||
| fs.removePerSubject(sm.subj) | |||
| wasLast := fs.removePerSubject(sm.subj) | |||
There was a problem hiding this comment.
Since removePerSubject is fs scoped can just do this inside that function IMO..
There was a problem hiding this comment.
I did try that, but there's a couple cases where it doesn't make sense, i.e. in expireMsgs and expireMsgsOnRecover, we want to first make sure that the deleted message isn't itself a delete marker (so that we don't end up making more of them endlessly), but we still need to do removePerSubject regardless.
There was a problem hiding this comment.
But that can all be in removePerSubject() if we pass in bool yes that indicates if this is a marker?
There was a problem hiding this comment.
Have updated it so we pass in a bool on whether or not to queue a marker.
neilalexander
force-pushed
the
neil/purgemarkers
branch
2 times, most recently
from
1223b9b to
b0ddc7d
Compare
|
Have renamed the |
server/filestore.go
Outdated
| fs.subjectDeleteMarkerIfNeeded(sm, JSAppliedLimitMaxAge) | ||
| mb.mu.Lock() | ||
| } | ||
| fs.removePerSubject(sm.subj, len(getHeader(JSMarkerReason, sm.hdr)) == 0) |
There was a problem hiding this comment.
This a perf hit you think?
There was a problem hiding this comment.
It might be narrowly, so I've uplifted the config check up as you suggested.
Signed-off-by: Neil Twigg <neil@nats.io>
neilalexander
force-pushed
the
neil/purgemarkers
branch
from
afbbfad to
a647835
Compare
This PR adds subject delete markers when doing purge/compact/remove operations for the filestore.
Memstore will be a separate PR but expect it to look a lot like this one.
Remaining questions before I mark for review:
Nats-Applied-Limitmake sense or do we want a more generic name likeNats-Marker-Reason?PurgeandRemove?Signed-off-by: Neil Twigg neil@nats.io