Add support for subject delete markers by MaxAge
#6378
Conversation
server/filestore.go
Outdated
| } | ||
| // Build the system tombstone. | ||
| var _hdr [128]byte | ||
| hdr := fmt.Appendf(_hdr[:0], "NATS/1.0\r\n%s: %s\r\n%s: %s\r\n\r\n", JSAppliedLimit, reason, JSMessageTTL, fs.cfg.LimitsTTL) |
There was a problem hiding this comment.
LimitsTTL seems like wrong name IMO.
There was a problem hiding this comment.
Best that @ripienaar has input here as this is the name from the ADR.
There was a problem hiding this comment.
When limits are applied and tombstones are written, this is the TTL of those messages.
We didnt consider purges would be involved until later, do you have a name suggestion @derekcollison ?
There was a problem hiding this comment.
Naming is hard, so not at the moment but will give it some thought.
There was a problem hiding this comment.
something like ActionTTL or NotificationTTL? Other ideas? Failing to find a better name :)
There was a problem hiding this comment.
It feels weird still, if it's LimitsTombstones then never doesn't make sense for something that does leave TTLs, and if it's LimitsTTL or LimitsTombstonesTTL then it's difficult to explain why the option is named after TTLs when it has a secondary larger effect.
There was a problem hiding this comment.
We can add 2 fields, I am not super stuck to the idea or anything, just conscious of how many config options we have, though I agree making it awkward is also not good.
And then having to guess if the user just forgot to set a TTL or if they really want forever tombstones etc?
There was a problem hiding this comment.
I think the turning on should be a bool that is clear we will generate markers when a subject disappears, and that should have a default TTL. We can have a second that allows them to override or change the TTL for he markers, including a "never" if needed.
There was a problem hiding this comment.
We've gone with SubjectDeleteMarkers bool and SubjectDeleteMarkerTTL string.
The default if not specified will be 15 minutes if not specified, RI and I think this is a good value as it's well outside the recovery time for fetch/ordered consumer/client/route/gateway etc failures as well as even application restarts.
neilalexander
force-pushed
the
neil/limitsttl
branch
from
37d2f57 to
af9f7fc
Compare
neilalexander
changed the title
MaxAgeMaxAge
Signed-off-by: Neil Twigg <[email protected]>
neilalexander
force-pushed
the
neil/limitsttl
branch
from
af9f7fc to
c8b7c48
Compare
|
This ready for a second review? |
|
@derekcollison Please! |
This PR adds the
LimitsTTLfunctionality for system tombstones when a subject is removed viaMaxAge. The tombstone will have the headerNats-Applied-Limitand the specifiedLimitsTTLvalue as the TTL.Further limits and/or administrative actions to follow separately.
This also requires API version 1, cannot be set if TTLs are disabled and cannot be combined with a mirror configuration.
Signed-off-by: Neil Twigg [email protected]