Duy Nguyen [off-list ref] writes:
On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano [off-list ref] wrote:
quoted
Duy Nguyen [off-list ref] writes:
quoted
On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano [off-list ref] wrote:
quoted
Christian Couder [off-list ref] writes:
quoted
So what should we do if freshen_file() returns 0 which means that the
freshening failed?
You tell me ;-) as you are the one who is proposing this feature.
My answer is, we are not worse than freshening loose objects case
(especially since I took the idea from there).
I do not think so, unfortunately. Loose object files with stale
timestamps are not removed as long as objects are still reachable.
But there are plenty of unreachable loose objects, added in index,
then got replaced with new versions. cache-tree can create loose trees
too and it's been run more often, behind user's back, to take
advantage of the shortcut in unpack-trees.
I am not sure if I follow. Aren't objects reachable from the
cache-tree in the index protected from gc?
Not that I think freshening would actually fail in a repository
where you can actually write into to update the index or its refs to
make a difference (iow, even if we make it die() loudly when shared
index cannot be "touched" because we are paranoid, no real life
usage will trigger that die(), and if a repository does trigger the
die(), I think you would really want to know about it).
On Fri, Jan 20, 2017 at 2:00 AM, Junio C Hamano [off-list ref] wrote:
Duy Nguyen [off-list ref] writes:
quoted
On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano [off-list ref] wrote:
quoted
Duy Nguyen [off-list ref] writes:
quoted
On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano [off-list ref] wrote:
quoted
Christian Couder [off-list ref] writes:
quoted
So what should we do if freshen_file() returns 0 which means that the
freshening failed?
You tell me ;-) as you are the one who is proposing this feature.
My answer is, we are not worse than freshening loose objects case
(especially since I took the idea from there).
I do not think so, unfortunately. Loose object files with stale
timestamps are not removed as long as objects are still reachable.
But there are plenty of unreachable loose objects, added in index,
then got replaced with new versions. cache-tree can create loose trees
too and it's been run more often, behind user's back, to take
advantage of the shortcut in unpack-trees.
I am not sure if I follow. Aren't objects reachable from the
cache-tree in the index protected from gc?
I think the problem is loose objects created then gc run just before
they are referenced (e.g. index written down). But I think I may be
following a wrong road. If mtime is in fact to deal with race
conditions, applying the same idea here is wrong because we have a
different problem here.
Not that I think freshening would actually fail in a repository
where you can actually write into to update the index or its refs to
make a difference (iow, even if we make it die() loudly when shared
index cannot be "touched" because we are paranoid, no real life
usage will trigger that die(), and if a repository does trigger the
die(), I think you would really want to know about it).
--
Duy
On Thu, Jan 19, 2017 at 8:00 PM, Junio C Hamano [off-list ref] wrote:
Duy Nguyen [off-list ref] writes:
quoted
On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano [off-list ref] wrote:
quoted
Duy Nguyen [off-list ref] writes:
quoted
On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano [off-list ref] wrote:
quoted
Christian Couder [off-list ref] writes:
quoted
So what should we do if freshen_file() returns 0 which means that the
freshening failed?
You tell me ;-) as you are the one who is proposing this feature.
Before the above question I already had given my opinion about what we
should do.
There are the following cases:
- Freshening failed because the shared index file does not exist
anymore. In this case it could have been removed for a good reason
(for example maybe the user wants to remove all the shared index
files), or it could be a bug somewhere else. Anyway we cannot know and
the user will get an error if the shared index file that disappeared
is read from afterwards, so I don't think we need to warn or do
anything.
- Freshening failed because the mtime of the shared index cannot be
changed. You already in a previous email wrote that we shoudn't warn
if the file system is read only, and I agree with that, as anyway if
the file system is read only, the shared index file cannot be deleted,
so there is no risk from the current user. Also the split index mode
is useful to speed up index writing at the cost of making index
reading a little slower, so its use in a read only mode should not be
the primary way it is used.
So my opinion is that there are good reasons not to do anything if
freshening fails.
quoted
quoted
quoted
My answer is, we are not worse than freshening loose objects case
(especially since I took the idea from there).
I do not think so, unfortunately. Loose object files with stale
timestamps are not removed as long as objects are still reachable.
As the current index is read, which freshens its shared index file,
before a new index is created, the number of shared index files
doesn't go below 2. This can be seen in the tests changed in patch
19/21. So the risk of deleting interesting shared index files is quite
low in my opinion.
Also in general the split-index mode is useful when you often write
new indexes, and in this case shared index files that are used will
often be freshened, so the risk of deleting interesting shared index
files should be low.
quoted
But there are plenty of unreachable loose objects, added in index,
then got replaced with new versions. cache-tree can create loose trees
too and it's been run more often, behind user's back, to take
advantage of the shortcut in unpack-trees.
I am not sure if I follow. Aren't objects reachable from the
cache-tree in the index protected from gc?
Not that I think freshening would actually fail in a repository
where you can actually write into to update the index or its refs to
make a difference (iow, even if we make it die() loudly when shared
index cannot be "touched" because we are paranoid, no real life
usage will trigger that die(), and if a repository does trigger the
die(), I think you would really want to know about it).
As I wrote above, I think if we can actually write the shared index
file but its freshening fails, it probably means that the shared index
file has been removed behind us, and this case is equivalent as when
loose files have been removed behind us.