Race condition upon queue flushing for an update

[HermannDppes]

Description

tl;dr

When upgrading to a different version, either there is no way without race conditions to flush the queues (and such a way needs to be created) or there is such a way (and it needs to be documented).

Expected Behaviour

When performing an upgrade that requires flushed queues (e.g. from 1.16.3 to 1.16.4), I can flush the queues without worrying they might be unflushed.

Actual Behaviour

The only documented way to flush queues I could find screams race condition so loudly that people started commenting “RIP headphone users” on unrelated YouTube-Videos.

Steps to Reproduce

This is, of course, only an example, but it was my scenario:

1. Run Gitea version 1.16.3
2. Decide to upgrade to a later version and see that this requires flushing the mirror sync queue
3. Find only gitea manager flush-queues in the documentation
4. Find out that it can be executed only when Gitea is running
5. Execute the queue flushing command and race to stop the Gitea instance (essentially gitea manager flush-queues && systemctl stop gitea)
6. Perform the upgrade
7. Sit back and acknowledge that what you just did feels really dumb because race conditions you neither know how to check for nor how to correct for should absolutely never be part of your process

Conclusion

So, if there was (for some reason or another) no race condition going on or I could have somehow prevented race conditions from occurring, I would have loved to know about that. As it stands, my best guess is that there was a race condition, there was nothing I could do about it and I got lucky and it caused me no problems. But as queue flushing is presumably here to stay as an occasional upgrade requirement, I'm definitely not satisfied with this state of affairs.

Gitea Version

e.g. 1.16.3 → 1.16.4

How are you running Gitea?

By reading documentation and release notes. :P

Less tongue-in-cheek, from binary via systemd, but this feels relevant to all variants of running Gitea.

[zeripath]

Sarcasm is not helpful.

If flushing queues could be done without Gitea running it would be done that way. It cannot and you should be able to work out why.

I wrote a PR that would allow one to interrogate the level db queues #18732 but it has not gained traction.

The logs report if the queues are empty on shutdown.

You only need to have empty queues if you're doing a major upgrade - but even then it's not absolutely necessary most of the time.

[HermannDppes]

Sarcasm is not helpful.

I'm sorry, I did not mean to cause offense. The occasional bit of humour helps me when writing technical texts and I, personally, appreciate it also when reading other's texts but I obviously overstepped what is considered sensible here and will accordingly stop doing this.

If flushing queues could be done without Gitea running it would be done that way. It cannot and you should be able to work out why.

I actually can not work out why. Though since you tell me it is impossible, I'll gladly take your word for it.

I wrote a PR that would allow one to interrogate the level db queues #18732 but it has not gained traction.

That definitely seems like a good doctor command to have and I see it has seen more activity again since you mentioned it.

The logs report if the queues are empty on shutdown.

That is good to hear, but I do have two follow-up questions:

1. If I do understand you correctly, the following procedure would work to downgrade the impact of the race condition to making “How long does it take to flush queues and shut down?” random as opposed to making “Will the queues be flushed after shutdown?” random.

   1. Execute gitea manager flush-queues && stop gitea
   2. Check logs
   3. If (relevant) queues are empty, exit
   4. Otherwise, start gitea
   5. Go to step (i.)

   The question is: Have I understood you correctly?

2. What is it for which I need to look in the logs? And is it tied to some log-level I would need to configure high enough? I do find these notes distributed among various shutdowns (in my testing so far, at least one of them per shutdown):

   ...eue/queue_channel.go:98:func1() [W] ChannelQueue: mail-channel Terminated before completed flushing
   ...eue/queue_channel.go:98:func1() [W] ChannelQueue: notification-service-channel Terminated before completed flushing
   ...eue/queue_channel.go:98:func1() [W] ChannelQueue: push_update-channel Terminated before completed flushing
   ...eue/queue_channel.go:98:func1() [W] ChannelQueue: task-channel Terminated before completed flushing
   

   Are these the log items telling me that some queue has not been empty at shut down? Or are there other indicators I have missed?

You only need to have empty queues if you're doing a major upgrade - but even then it's not absolutely necessary most of the time.

As long as the release notes only state “This PR will make old queue entrys unredable for the new version, make sure to flush the mirror sync queue before updating.” without comforting words that I'll easily notice and recover-from not having flushed the queues, I'll definitely sleep easier if I knew instead of hoped the queues to be empty.

[wxiaoguang]

The queue code has been completely rewritten. Usually you do not need to do "flush-queues", it's not a well-designed behavior.