Arvados

If we decide not to fix it we should at least add a 2.5.0 release note to highlight the impact on container reuse.

19981-reuse-flex-keep-cache @ 599e2e6798a2497ed56b6dfe917a09260dd74407 - developer-run-tests: #3530

Since I was just thinking about container reuse in #16325 and this was bugging me, I figured I could knock it out real quick. lol famous last words.

Runtime constraints are stored as text in the database, so the way this all works now is Container.find_reusable normalizes the hash, MD5's it, then looks for matches that way. The hashing precludes being selective about constraints.

Even if we undid the hashing, which would be a big change by itself, the only option we have to ignore specific constraints is by loose text matching, which I basically never want to do.

IMO we should convert this column to jsonb and then do rich matching on it, but that's obviously a bigger story that needs more discussion and agreement up front.

So, I have a branch up that generates variations of the runtime constraints by filling in common values for the Keep cache constraints, and considers them all eligible for reuse.

This does expand reuse in the direction that we want. It's also something that we're basically doing already: it's the strategy that we're using to look for containers that predate cuda and keep_cache_disk, just expanded to more values conceptually.

The main con of this approach is it doesn't even resolve the original bug. The real issue here isn't that the API server can't find containers that predate keep_cache_disk; it's that the configuration of DefaultKeepCacheRAM changed. Old containers used keep_cache_ram>0, and after the change new requests have keep_cache_ram=0, and that precludes a match. And it still does, even after this branch.

The cheapest way to address that would be to give administrators a configuration knob for "previous keep_cache_ram defaults to search for." With this branch, it would be easy to include those values in the search. But that also requires more discussion, and maybe it would be better to just rip the band-aid off and make a bigger change like the jsonb switch.

On balance, I think, I hope, that this branch is still a net win. But it's admittedly a lot more marginal than I hoped it would be when I started working on it.

This branch LGTM, thanks.

For a longer term solution, another possibility -- less flexible but more efficient than rich matching on json -- is an explicit reusable_runtime_constraints (or just ..._md5) column, with only the keys we really want to match on. When we change the rules, this column could be rewritten in a migration.

(We could almost just rewrite runtime_constraints for old containers, but I think that would be too obnoxious since it would no longer serve as the record of what was actually done.)

Tom Clegg wrote in #note-15:

For a longer term solution, another possibility -- less flexible but more efficient than rich matching on json -- is an explicit reusable_runtime_constraints (or just ..._md5) column, with only the keys we really want to match on. When we change the rules, this column could be rewritten in a migration.

This makes sense to me too and I would be fine with it.

Since my branch doesn't actually fix the bug, I have updated the subject following what I learned, and the proposed fix to capture these ideas.

(We could almost just rewrite runtime_constraints for old containers, but I think that would be too obnoxious since it would no longer serve as the record of what was actually done.)

I agree this would be user-hostile. People want to go back and look up "what was the X setting I used on the container that worked?" so we don't want to lose that information.