On Tue, Jul 11, 2023 at 05:50:50PM +0200, Omar Polo wrote:
> On 2023/07/11 17:27:05 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > On Tue, Jul 11, 2023 at 05:15:58PM +0200, Omar Polo wrote:
> > > similar to what we do for the loose objects.  This prevents races when
> > > gotadmin load, got fetch or gotd are installing a new pack file and a
> > > concurrent gotadmin cleanup wants to delete it as redundant because it
> > > hasn't seen the new refs.
> > 
> > > @@ -1203,6 +1204,17 @@ purge_redundant_pack(struct got_repository *repo, cons
> > >  		return got_error(GOT_ERR_NO_SPACE);
> > >  
> > >  	/*
> > > +	 * Do not delete pack files which are younger than our maximum
> > > +	 * modification time threshold.  This prevents a race where a
> > > +	 * new pack file which is being added to the repository
> > > +	 * concurrently would be deleted.
> > > +	 */
> > > +	if (fstatat(got_repo_get_fd(repo), path, &sb, 0) == -1)
> > 
> > Shouldn't this ignore ENOENT?
> 
> not sure; `path' is the path to an .idx file we've opened and parsed
> already so if it disappeared it could be a hard error.

We ignore ENOENT for other paths in the looop below as well.
Since this function is supposed to remove the files (under some
conditions) if they were removed already then the job has been
done and we can simply ignore them.

> does it read better with the ENOENT handling?

Yes, ok.