Add the ability to control which files in the input root are hardlinked #217
Conversation
EdSchouten
force-pushed
the
eschouten/20220412-hardlink
branch
from
2ff088a
to
a9403a5
Compare
This change acts as a counter proposal to PR bazelbuild#216. Instead of globally stating whether files should be hardlinked or not, it adds the ability to let clients control whether files are aliased. It achieves this by adding a salt field to FileNode. If two files have the same salt and all other (non-name) properties match, they SHOULD get constructed in the form of hardlinks.
EdSchouten
force-pushed
the
eschouten/20220412-hardlink
branch
from
a9403a5
to
ba47739
Compare
This would mean that by default (empty salt), all files with the same digest (and attributes) should be hardlinks to the same inode. In my opinion, this is not a good default. I'd be fine with allowing such files to be hardlinks if the implementation requires this to be efficient, but I don't think this is the most desirable default. This would be especially problematic if files are mutable. Appending to a mutable empty file would modify all other empty files. Or did you mean to apply this rule only for files that have a non-empty salt? |
That's open for discussion. I'd be fine with that. |
| // Salt to force that files in the input root have distinct inode | ||
| // numbers, even if all properties match. | ||
| // | ||
| // Two files inside the input root SHOULD have the same inode number |
There was a problem hiding this comment.
What does it mean for this to be a SHOULD ? Is the server free to ignore it? Or if not, what are the implications for doing so?
I'm also wondering how it would compare for your use-case to add a HardlinkNode in parallel to SymlinkNode, instead of embedding it into FileNode as you do here. That sidesteps the configuration confusion of having to exactly duplicate the digest/is_executable/node_properties.
Now, if you really do want inode sharing to be a soft recommendation rather than a requirement, that may be a poor fit. But I'm thinking it might be better to handle that via Capabilities negotiation and having the client fall back if willing (or error if not) instead of being uncertain if nodes were in fact hardlinked or not?
This change acts as a counter proposal to PR #216. Instead of globally
stating whether files should be hardlinked or not, it adds the ability
to let clients control whether files are aliased.
It achieves this by adding a salt field to FileNode. If two files have
the same salt and all other (non-name) properties match, they SHOULD get
constructed in the form of hardlinks.