Pulp

I'll take a closer look and make a more specific proposal. I don't think we should allow an individual task to do hundreds of DB writes per second when one or two would suffice. It's hard to imagine that not impacting performance, especially with multiple tasks running concurrently.

Sounds good. Skipping the writes is easy enough, I think the crux of the issue is ensuring that a save() will occur later if some of the most recent saves (or just the very last one) was skipped. I don't know how to do that without a separate thread, or epoll callback loop, etc.

Maybe where we are going wrong is that we are trying to hide this from the plugin writer. Note they could just save less often and they can ensure correctness.

What about this as an idea, we could make it do this fancy behavior only if it is being used inside the context manager.

To be clear, I do not want to hide this behavior at all. I do want to choose default behaviors that make the most common use cases easy, but we should be very clear about this behavior and possibly allow the plugin writer to influence it.

I think we can wrap this save call in a little bit of logic that only saves at most once per some interval of time:

https://github.com/pulp/pulp/blob/f44599980274ec4cf69594f84882ca90273041cf/app/pulp/app/models/progress.py#L208

As you point out, we could only do the batching in the case where the object is being used as a context manager. That's a reasonable approach that I would be happy with.

To allow more control, we could optionally have a boolean property of the object, "batch_increments" or something like that, which defaults to False. A user could set it to True if they want, and it would also be set to True in "__enter__". This is probably overkill for a first cut, but it's an option for the future in case people want to do incrementing without using the object as a context manager.

I also think it would be fine to just do the batching in the increment() method, make sure that if "done == total" a save always happens, and clearly document that the user is responsible for doing a final save on the object when they are done incrementing it.

Any of these approaches is fine with me, but I do think we need to solve this for pulp 3. Can we add the pulp 3 and sprint candidate options back to this refactor?

PR: https://github.com/pulp/pulp/pull/2986

I removed `increment() skip-save logic` from the checklist, since increment() still calls save() because https://pulp.plan.io/issues/2318 was resolved via documentation.