Cache settings more aggressively in web client

[Jeff Davis]

Workstation settings are now stored on the server (bug 1750894), which is great. However, the client is now doing an awful lot of calls to the server to lookup workstation/user/library settings. Where possible, we should cache settings locally to avoid the extra traffic.

For example, yesterday our 3.3.4 environment saw about 200,000 calls to either open-ils.actor.settings.retrieve.atomic or open-ils.actor.ou_setting.ancestor_default.batch via the gateway. (That's 40% of all open-ils.actor calls. To handle the extra traffic compared to 3.1.7, we have increased open-ils.actor drones from 50 to 125 per server and are waiting to see if that will suffice.) About 250 different settings were looked up; here are the ones that had >10,000 lookups:

lib.timezone: 24,412
ui.staff.angular_catalog.enabled: 24,289
ui.staff.max_recent_patrons: 24,289
webstaff.format.date_and_time: 24,289
webstaff.format.dates: 24,289
circ.staff_client.do_not_auto_attempt_print: 21,958
circ.clear_hold_on_checkout: 15,607
ui.staff.require_initials.patron_standing_penalty: 15,607
ui.admin.patron_log.max_entries: 15,416
ui.admin.work_log.max_entries: 15,416
format.date: 11,136
eg.audio.disable: 10,425

Most of these (timezone, date formats, eg.audio.disable) rarely change and could be looked up once at the start of the session and then cached. Others are more likely to be changed by staff during a session, but we could trigger a cache update when the user changes the value.

[Jeff Davis (jdavis-sitka)]

The largest numbers of lookups are attributable to just a few spots in the AngularJS client:

- The top 5 settings are almost always looked up together during egStartup.
- The next 5 settings are mainly looked up together immediately after startup for egCirc.
- format.date is looked up anew for every egDateInput.

From what I'm seeing in our logs, we could eliminate up to 45% of open-ils.actor.ou_setting.ancestor_default.batch calls by using cached values for those 11 specific settings in these three places.

[Jeff Davis (jdavis-sitka)]

Working branch user/jeffdavis/lp1848550-cache-settings leverages the offline settings cache to reduce org setting lookups in AngularJS:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1848550-cache-settings

A "cachedate" column is added to the Setting table in IndexedDB. This tells us how long ago a setting was last looked up from the server. If the cachedate is more than 12 hours old, the cached value is ignored and the current value is retrieved from the server instead, which triggers an update of the offline cache value. (For offline, since we can't refresh from the server, we ignore the cachedate and use whatever value we've got.)

This approach means that org setting values could be up to 12 hours out-of-date in the client. I think that's a reasonable tradeoff since org settings rarely change, and at worst you can just clear local data to force a refresh. Alternatively, we could use a smaller interval, make it configurable, or add a server-side mechanism to force a cache refresh.

Most setting-related open-ils.actor traffic appears to be coming from AngularJS, but it would make sense to do the same thing in Angular once we've agreed on an approach.

[Bill Erickson (berick)]

Jeff, this approach seems reasonable to me.

A useful addition would be a new "Cached Org Settings" tab in the Workstation Admin => Stored Preferences page. This would allow users to clear specific values. A "Clear All" button might be good too. We could also wait to implement this as part of the Angular port of stored prefs UI so we don't have to code it twice. If that makes sense, I'll open a separate LP for porting the stored prefs UI.

[Galen Charlton (gmc)]

I do think it's important to be able to have some way for the server to invalidate cached library settings, since when one of them is changed, it's often with the expectation that the change will take effect immediately. Even 12 hours could be too long in some cases.

Now, "immediate" is a relative thing; I'm not necessarily proposing that the client run a heartbeat to check for cache-invalidation every minute. However, I do think there needs to be something that the user can do to guarantee that they're looking at fresh values, such as the "Clear All" button that Bill suggests and doing a cache check (or invalidation) upon logging in to the staff client.

[Jeff Davis (jdavis-sitka)]

Clearing the cache on successful login makes the most sense to me. After login, the latest values would be fetched from the server the next time they're needed. When necessary, staff can force a refresh via logout-then-login, which feels simple and intuitive. Bill's suggestion would be nice to have, but I'd count it as a separate feature.

[Bill Erickson (berick)]

+1 to clearing on successful login. We may create smarter, longer-term caching options down the road, but this is a step in the right direction and will certainly reduce the number of settings API calls. That opens the door to simplifying the patch, too, since we no longer have to store the cachedate.

[Jeff Davis (jdavis-sitka)]

Working branch user/jeffdavis/lp1848550-cache-settings-no-cachedate has an attempt at an implementation:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jeffdavis/lp1848550-cache-settings-no-cachedate

It works for me in initial testing (settings are cached upon retrieval, cache is cleared upon login) but needs to be vetted more thoroughly.

Once we have something that is confirmed to work, I would like to suggest that this be treated as a bugfix rather than a new feature.

[Jeff Davis (jdavis-sitka)]

One problem I'm seeing is that we have separate login flows for Angular and AngularJS (bug 1835128). Since Angular is not Lovefield-aware, login via /eg2/en-US/staff/login will not be able to clear settings from the Lovefield-based offline cache.

The Angular client will likely cache settings in window.localStorage, and it should be possible for AngularJS login to clear that out. If we fix bug 1835128 by forcing the use of AngularJS login everywhere, including in the Angular client, we should be able to clear cached settings properly.

[Jeff Davis (jdavis-sitka)]

I've pushed an additional commit which redirects the Angular login page to its AngularJS equivalent, as proposed in my previous comment.

[Bill Erickson (berick)]

Since the Angular code will need the ability to relay requests to our IndexedDB handler, I'm also going to start working on that. I'll open a separate bug.

Fortunately, we don't need to reimplement the lovefield/indexeddb parts, just the shared worker pass-through. It should be a pretty direct port. It's possible implementing the cache clearing can be done quickly, in which case we could use it instead of the login redirect, but I also don't want to hold up progress on this bug.

[Bill Erickson (berick)]

Scratch the part of my comment about skipping the login redirect. I forgot there was more to it than just the cache clearing.

[Bill Erickson (berick)]

For reference, bug #1854850 (Angular IndexedDB communication service)

[Bill Erickson (berick)]

See https://bugs.launchpad.net/evergreen/+bug/1854850/comments/1 for angular implementation. I'm leaving this as a separate bug, since part of that code will need to be merged regardless of this bug.

[Jeff Davis (jdavis-sitka)]

Since applying the fix in production a week ago, we have seen a 90% reduction in open-ils.actor.ou_setting.ancestor_default.batch calls, and a 10% reduction in open-ils.actor calls overall. I don't have metrics for network traffic, but fewer API calls seems like a good thing.

Each of our four single-server "bricks" has open-ils.actor max_children = 150. Prior to the fix, we were hitting that limit every day. Since the fix, the open-ils.actor drone count has been much lower, exceeding 75 only once. Our servers also show somewhat improved memory usage (committed memory is lower and more consistent), although I can't definitively attribute that to the fix.

[Bill Erickson (berick)]

Thanks, Jeff.

I've pushed a new branch with a couple of additions/changes:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1848550-cache-settings-no-cachedate

1. Sign-off for Jeff's AngJS caching code.

2. Merged in equivalent code for the Angular side (from bug #1854850) -- If we're doing one, we really need to do both.

3. I tweaked the redirect logic to redirect post-login to the AngJS splash page, in lieu of a pre-login redirect to the AngJS login page.

I did this in part because it's replacing a redirect instead of adding a new one. I'm also keen to give the Angular login page plenty of use in the wild and my understanding from bug #1835128 is that splash page consistency is the main concern, not the login page. However, if we really would rather lock down the login page as well, I don't have any big objections to that. Just note the redirect would have to be disabled to test the above Angular code.

[Chris Sharp (chrissharp123)]

Signoff branch pushed: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1848550-cache-settings-no-cachedate

We've been running Jeff's and Bill's changes in PINES production for a couple of weeks now with no problems (granted, most of our libraries are closed, but some are open today and all is fine).

[Bill Erickson (berick)]

Thanks, Chris. I gave it a another run through and things are looking good. Merged to 3.5 / master.