Topic on Talk:Requests for comment/SessionStorageAPI

Jump to navigation Jump to search

Values persist until expiring, the result of a TTL configured on a site-wide basis.

13
Pchelolo (talkcontribs)

You write 'site-wide basis'. This is ambiguous. Are we talking about 'site' as wiki-specific TTL? Are we talking global TTL for the whole 'site'? If that's the former - I don't see how that could be possibly implemented

EEvans (WMF) (talkcontribs)

Site-wide here is used here in the context of the session storage service, so it means that the service is configured to expire records after a TTL period, and everything stored uses that TTL. Is that behavior wrong? If not, and that was just unclear, do you have suggestions for how to make that more obvious?

EEvans (WMF) (talkcontribs)

To update this thread with discussion that has occurred elsewhere: a site-wide configuration (in the sense that everything is stored with a TTL specified in the service's configuration), might not be the way to do this. Though it is not apparent this is needed at the WMF, setting a TTL on the persistence is a part of the contact, vis-a-vis Manual:$wgObjectCacheSessionExpiry, (or via Manual:Hooks/SessionMetadata).

It may be necessary that the service accept an attribute for the TTL on write.

Tgr (WMF) (talkcontribs)
or via Manual:Hooks/SessionMetadata

I misread that part of the code, sorry. The hook cannot change existing keys in the metadata array (including expiration time), it can only add new ones.

Pchelolo (talkcontribs)

It seems like the wgObjectCacheSessionExpiry is a no-op by now as well, reopened task T158829 to figure out whether my conclusion is correct and deprecate it.

Mobrovac-WMF (talkcontribs)

We have global user settings, so "site-wide" should mean "all SUL wikis" here. It doesn't make sense to have per-wiki TTLs, because we don't have per-wiki users.

DKinzler (WMF) (talkcontribs)

I don't think we have to support per-write TTLs for sessions. We shouldn't let the BagOStuff interface influence the design of the service interface.

Anyway, I think "site-wide" is confusing. "Per-instance" makes more sense in my mind. "Site" is not a concept that is well defined in the context of this service. An instance of the service is.

EEvans (WMF) (talkcontribs)

Anyway, I think "site-wide" is confusing...

I've clarified this in the text.

Anomie (talkcontribs)

In Topic:Uoge9eeznt3nntm0 I analyzed the requirements of things using the existing Redis session store, and it seems to me that at least OAuth probably requires client-specified TTLs.

Or rewriting to use a different store, or having SessionManager confusingly not use $wgSessionCacheType anymore.

Pchelolo (talkcontribs)

I believe there was a consensus on the RFC meeting that client-supplied TTL feature is required.

Regarding the API for the feature, we could either require a Cache-Control: max-age <TTL> header, or include the TTL as a required property into the payload. I personally slightly prefer the former, as I'm personally not yet convinced we need to wrap the actual data into a JSON with a single value property.

Also, SMalyshev have raised a question of what happens if for some reason someone forgets about the data, or (if we require the Cache-Control header) - sets it to close-to-infinite TTL - then the data will be there forever. So, I believe apart of the client-supplied TTL we need to set service-wide default TTL and cap the client-supplied TTL to it. Something significantly higher then we use now (which is 3600s)

Tgr (WMF) (talkcontribs)

At a glance, the longest expiry we currently use is one day, for CentralAuth global sessions.

Mobrovac-WMF (talkcontribs)

It was evoked in the IRC meeting that client-supplied TTLs might be needed, but I'm not really persuaded that is true. Even if we do allow per-request TTls, these should be taken into account only if they are lower than the configured, default TTL, IMHO.

Pchelolo (talkcontribs)

> It was evoked in the IRC meeting that client-supplied TTLs might be needed

To quote the RFC meeting,

22:08:23 <TimStarling> the only thing missing is client-side specification of the TTL

22:26:14 <TimStarling> also my strong preference is for there to be client-side TTL specification

22:28:35 <_joe_> SMalyshev: the proposed interface has a server-side TTL

22:35:09 <_joe_> can we say we have a consensus on client-set TTLs to be a desirable feature?

22:35:54 <SMalyshev> yes. q: is client TTL still subject to server one-fits-all ttl or replaces it?

22:36:20 <tgr> if the service is implemented as a new BagOStuff, which seems to be the easy way, it should honor the BagOStuff contract, and that includes TTL

22:36:21 <TimStarling> one reason I don't like server-side TTL is because configuring services is a PITA

22:36:25 <_joe_> mobrovac: but we want the clients to be able to set a LOWER ttl for a specifc session

22:36:51 <tgr> and adding TTL is easy, so really it seems less effort to support it than not

22:37:25 <_joe_> so we need client-side TTLs :)

Sounds like a consensus to me :)

Reply to "Values persist until expiring, the result of a TTL configured on a site-wide basis."