Thank you @bzikarsky.

Approach looks ok. But to me this seems to be a BC break. Old entries don't have the expiry field and thus GC doesn't work for them. Or am I missing something?

You are right, currently old (pre-patch) sessions wouldn't even be cleaned up. No one thought of this yet. Oh well.

We can now fix:

a) only gc() to garbage-collect all sessions without expiry -- pre-patch session get timed out immediately, but we don't have a lot of "BC baggage"

b) gc() to garbage-collect sessions without expiry in the old way and change read() to assume all sessions without expiry to be valid (old sessions are treated exactly the same as before, which we considered buggy)

c) gc() as in b), but read() to validate sessions without expiry to the same rules as gc() did before the patch. Essentially valid session-age would be enforced on all new and old sessions. This solution has probably the highest overhead to ensure BC.

I'd go with a) personally. It's very lightweight and I think a framework upgrade allows for sessions to be invalidated.

What do you think? (/cc @jmikola)

My understanding is that all three options would require $or logic in the gc() remove criteria. In (a), we would be removing all documents where expiry_field is out of range or it doesn't exist. I was initially concerned that the existence check wouldn't use an index, but I was reminded that it was only a temporary bug and has since been fixed (SERVER-10608).

For (b) and (c), which use the same gc() logic, we would still need $or logic. The clauses would be an expiry_field out of range or the time_field range check combined with expiry_field not existing. The second clause seems less performant, especially if users won't be indexing time_field. Based on the current code, time_field isn't event used, so I'm not sure why we're even still storing it (perhaps just for metadata, since _id is not an ObjectId that includes the creation timestamp). I'm not suggesting we remove time_field, but I don't think we should query on it.

That said, I'm in favor of (a). If we must use $or clauses in the remove criteria (effectively two queries), (a) at least relies on a single index we assume will exist. If we are concerned with sessions being invalidated during upgrades, it would be trivial to draft a script that, given a lifetime integer, can migrate documents in the sessions collection and create the necessary expiry_field values based on the time_field values. We could include that in release notes if we think folks would actually take notice and make use of it.