On Mon, Nov 11, 2024 at 10:06:10AM -0800, Calvin Wan wrote:
> On Sun, Nov 10, 2024 at 11:07 PM Patrick Steinhardt <ps@xxxxxx> wrote:
> > > [TASK_LOOSE_OBJECTS] = {
> > > "loose-objects",
> > > maintenance_task_loose_objects,
> > > loose_object_auto_condition,
> > > + SAFE,
> > > },
> > > [TASK_INCREMENTAL_REPACK] = {
> > > "incremental-repack",
> > > maintenance_task_incremental_repack,
> > > incremental_repack_auto_condition,
> > > + SAFE,
> > > + },
> > > + [TASK_UNSAFE_GC] = {
> > > + "unsafe-gc",
> > > + maintenance_task_unsafe_gc,
> > > + need_to_gc,
> > > + UNSAFE,
> > > + 0,
> > > + },
> > > + [TASK_SAFE_GC] = {
> > > + "safe-gc",
> > > + maintenance_task_safe_gc,
> > > + need_to_gc,
> > > + SAFE,
> > > + 0,
> > > },
> >
> > Hm. I wonder whether we really want to expose additional tasks to
> > address the issue, which feels like we're leaking implementation details
> > to our users. Would it maybe be preferable to instead introduce a new
> > optional callback function for every task that handles the pre-detach
> > logic?
>
> This does sound like a good idea. However, would there be any issue
> with running all pre-detach logic before running post-detach logic?
> I'm thinking if pre-detach logic from a different function could
> affect post-detach logic from another. If not, I do agree this would
> be the best solution going forward.
Sure, in theory these can interact with each other. But is that any
different when you represent this with tasks instead? The conflict would
still exist there. It's also not any different to how things work right
now: the "gc" task will impact the "repack" task, so configuring them
both at the same time does not really make much sense.
Patrick
