• fix: serialize updateEntitlements() #14974 👈
• main

This stack of pull requests is managed by Graphite.Learn more about stacking.

Join@spikecurtis and the rest of your teammates onGraphite

Just so I understand, and jotting notes as I break for a minute. I am going to play with this after some morning calls.

Update (which wasReplace) does an entire entitlements replacement. The multiple go routines callingupdateEntitlements all source from the license in thedb.

Is the issue that in the multiple calls in unit tests, the earlier calls do not have a license. This earlier call happens in parallel to the later call (with license). If the later one finishes first, then the earlier call overwrites it back to no license.

The fix, is to force serialization such that the order of calls toupdateEntitlements is respected. And because the db lookup happens withinupdateEntitlements, if the earlier execution in the previous case is delayed, it's db lookup is also delayed. So it will fetch the latest value.

This looks to be a good solution. The test does not exercise the bug when I remove the channel, but I only briefly looked at it atm.

An aside. when I extracted this logic out into this package, I was not really happy with the API it exposed. But it was already used throughout the codebase, and refactoring it everywhere felt like a large lift.

Since we only have 1 entitlement per coderd, it could probably be it's own go routine and take feature sets to merge into the existing. Ensuring everything is done in order if multiple threads are working with it. Order would matter still though, as there is no time information to know whichFeature should take precedence.

Is the issue that in the multiple calls in unit tests, the earlier calls do not have a license. This earlier call happens in parallel to the later call (with license). If the later one finishes first, then the earlier call overwrites it back to no license.

Yeah, exactly.

The test does not exercise the bug when I remove the channel, but I only briefly looked at it atm.

Hard to hit the bug consistently, since it's a race over which one writes first.