make it possible to cap event retention time

[richvdh]

currently it is possible to configure default event retention via the retention.default_policy settings (https://github.com/matrix-org/synapse/blob/master/docs/sample_config.yaml#L392), but this can be overridden by m.room.retention events.

allowed_lifetime_{min,max} prevent users on the local server sending m.room.retention events which exceed the bounds, but do little to counter events generated on remote servers.

[richvdh]

It would be good to understand why the current implementation was chosen. I think ideally allowed_lifetime_{min,max} would cap any m.room.retention events.

[babolivier]

It would be good to understand why the current implementation was chosen.

I think this is because it's a direct port of the DINUM implementation. Given they're running through a closed federation, where allowed_lifetime_{min,max} can be set to the same values to every server, capping the values for new local events seemed good enough.

Ideally, the implementation should be updated so it deletes events on max(allowed_lifetime_min, m.room.retention/max_lifetime) (i.e. "delete after the maximum duration in the room's state except if the server mandates to keep the event longer") and doesn't delete events before min(allowed_lifetime_max, m.room.retention/min_lifetime) (i.e. "don't delete before the minimum duration in the room's state except if the server mandates to delete the event after a shorter duration").

[richvdh]

thanks @babolivier. I guess there's no harm in having allowed_lifetime_{min,max} apply as a cap both on the events that can be generated by local users, and on the actual retention time when we come to do a purge.

[richvdh]

see also #7693

[babolivier]

I'm currently looking at this; it looks like the current implem checks local events against allowed_lifetime_{min,max} in the EventValidator, which means remote events that don't match the local config will be rejected - which makes sense for a closed federation but not otherwise.

So technically saying that we "do little to counter events generated on remote servers" is wrong, because we reject events that disagree with the local config - which is probably worse.

I'll move these checks somewhere else, and use these config settings in two distinct ways:

• reject events sent by local users that don't implement the bounds from the config actually let's treat them like events received over federation in that regard, i.e. accept them
• correct the values when loading policies from the database