server: Default value of MaximumMessageExpiryInterval

@dadebue Hope you feel better soon! Remember to get plenty of rest and water!

❤️ I’m sorry to hear that . Take your time to recover, and we can address the issue once you’re feeling better.

@dadebue Could you take a look at the Unresolved conversations in PR #315 ?

I was struggling with a cold for a few days…

I’ve reviewed and added a comment…

@dadebue Refer to Create a pull request

This needs some more thought! I tried setting the MaximumMessageExpiryInterval values to 0 or math.MaxInt. And in both cases the retained messages are cleared right away.

The problem lies in line 1600 of server.go:

if (pk.Expiry > 0 && pk.Expiry < now) || pk.Created+s.Options.Capabilities.MaximumMessageExpiryInterval < now {

The second condition is always true for 0 and Math.MaxInt:

• for 0 it’s true because Created is always smaller than now
• for math.MaxInt it’s true because of Go integer overflow (variable wraps around and becomes negative and therefore is smaller that now)

I suggest something like this (variable declaration just for readability):

// clearExpiredRetainedMessage deletes retained messages from topics if they have expired.
func (s *Server) clearExpiredRetainedMessages(now int64) {
	if s.Options.Capabilities.MaximumMessageExpiryInterval == 0 || s.Options.Capabilities.MaximumMessageExpiryInterval == math.MaxInt {
		return
	}

	for filter, pk := range s.Topics.Retained.GetAll() {
		maxExpiry := pk.Created + s.Options.Capabilities.MaximumMessageExpiryInterval
		packageExpired := pk.Expiry > 0 && pk.Expiry < now
		forceExpiry := maxExpiry > 0 && maxExpiry < now

		if packageExpired || forceExpiry {
			s.Topics.Retained.Delete(filter)
			s.hooks.OnRetainedExpired(filter)
		}
	}
}

@dadebue This is a very good point. I’ve added it to the README - let me know if it’s unclear or needs improvement 🙂 Thank you for raising it!

Hi @dadebue!

This is correct - math.MaxInt allows an effectively infinite number of retained and inflight messages to queue up on the server. In a hostile network environment, this can be used to DOS the broker, and we didn’t feel comfortable releasing with this as the default configuration. For this reason we opted for a ‘safer’ default value.

You can still configure it to use math.MaxInt, or some other value, however.🙂

Happy to discuss increasing this default value, too.