Re: [PATCH 08/17] odb/source: make `close()` function pluggable

[Patrick Steinhardt <ps@pks.im>]

On Thu, Mar 05, 2026 at 10:58:32AM +0000, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > diff --git a/odb/source.h b/odb/source.h
> > index 2f8132f9e1..7af4900ab4 100644
> > --- a/odb/source.h
> > +++ b/odb/source.h
> > @@ -59,6 +59,14 @@ struct odb_source {
> >  	 */
> >  	void (*free)(struct odb_source *source);
> >
> > +	/*
> > +	 * This callback is expected to close any open resources, like for
> > +	 * example file descriptors or connections. The source is expected to
> > +	 * still be usable after it has been closed. Closed resources may need
> > +	 * to be reopened in that case.
> > +	 */
> 
> Nit: here we say 'may' need to be reopened...
> 
> > +	void (*close)(struct odb_source *source);
> > +
> >  	/*
> >  	 * This callback is expected to clear underlying caches of the object
> >  	 * database source. The function is called when the repository has for
> > @@ -104,6 +112,16 @@ void odb_source_free(struct odb_source *source);
> >   */
> >  void odb_source_release(struct odb_source *source);
> >
> > +/*
> > + * Close the object database source without releasing he underlying data. The
> > + * source can still be used going forward, but it first needs to be reopened.
> > + * This can be useful to reduce resource usage.
> > + */
> 
> Here, we're more explicit that it does need to be reopened. I like the
> latter better, this way, sources which don't need to be re-opened can
> simply do a no-op. But this makes the expectation on the user side more clear.

I consider the first comment to be catered towards the developer of a
backend, whereas the second comment is catered towards the user of these
interfaces. So I'm intentionally being a bit more lose on the first one
as we cannot assume how exactly the backend is implemented, and whteher
it even needs to open anything. For the end user though they should
treat this as if we were always reopening.

Patrick