forceDerivation(): Wait for async path write after forcing value#176
forceDerivation(): Wait for async path write after forcing value#176
Conversation
If the drv attribute exists in the eval cache but the drv does not exist in the Nix store, then the drv may not exist immediately after the call to forceValue(). So we have to synchronize there.
| auto aDrvPath = getAttr(root->state.sDrvPath); | ||
| auto drvPath = root->state.store->parseStorePath(aDrvPath->getString()); | ||
| drvPath.requireDerivation(); | ||
| root->state.waitForPath(drvPath); |
There was a problem hiding this comment.
I'm not really qualified to review this, but I'm curious - why did you move this line and not duplicate it?
Is there no chance that requireDerivation might also require waiting, and would otherwise avoid having to forceValue if you had waited?
There was a problem hiding this comment.
See my other comment.
| auto aDrvPath = getAttr(root->state.sDrvPath); | ||
| auto drvPath = root->state.store->parseStorePath(aDrvPath->getString()); | ||
| drvPath.requireDerivation(); | ||
| root->state.waitForPath(drvPath); |
There was a problem hiding this comment.
What happens right now, with the waitForPath here? (I guess my real question is: what does this really fix? It seems like waitForPath is now being called in less cases than before, which may be fine and correct but seems weird for "calling it more times than necessary" to cause issues...)
There was a problem hiding this comment.
I understood it as the critical bit was needing to make sure there's a wait after calling aDrvPath->forceValue();. I'm likely hitting the case in the comment - the nix eval cache is shared between CI runs, and occasionally I manually run GC on the box.
There was a problem hiding this comment.
The original waitForPath() was basically pointless, because it waits before the attribute has been evaluated. So at that point, the asynchronous creation of store paths (the .drv file and its dependencies) hasn't even started yet.
In the common case where the attribute is cached and the corresponding .drv file exists in the store, there is no need to wait for anything. And if the .drv file has been GC'ed, we need to evaluate the attribute to recreate it and then wait.
the nix eval cache is shared between CI runs, and occasionally I manually run GC on the box.
Right, this bug triggers if the eval cache gets out of sync with what's in the store.
|
Added a test. |
edolstra
force-pushed
the
fix-eval-cache-async-path
branch
from
4b0ec85 to
07ed0a2
Compare
3 participants
Motivation
If the drv attribute exists in the eval cache but the drv does not exist in the Nix store, then the drv may not exist immediately after the call to
forceValue(). So we have to synchronize there.Fixes a bug introduced in #162.
Context