-
Notifications
You must be signed in to change notification settings - Fork 363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cli: New ban.cancel command #4052
base: master
Are you sure you want to change the base?
Conversation
dridi
commented
Feb 7, 2024
It takes the same command line arguments as the ban command.
tentative response: I can see why something like this would appear attractive to curtail the impact of an accidentally issued ban, but I do not feel confident to oversee the potential consequences, in particular with respect to persistence. @dridi which model do you have in mind? On IRC, you said "no effect on ban persistence", what does that mean? Would the canceled ban be still applied to persisted objects? Or would it not be applied? How would this look like? The most natural scenario, in my mind, would be that eventually the canceled ban would not get exported any more and thus become invisible to persisted objects, but until then, it could still get reloaded. So, consequently, would it not make sense to generate a "cancel" stv event? |
This strictly operates on the live ban list. There is no interaction with storage whatsoever. The effect is "immediate", pretty much identical to the current effect of the |
I don't understand how this is supposed to work then. If stevedores are not involved, then the ban applies and is not canceled when a persistent storage is reloaded at the righ^Wwrong time. The use case I have in mind here is to clone caches by snapshot/copying the storage, where this scenario is more likely than in the ordinary "we never restart our varnishes" case. |
For that, change the signature of BAN_Cancel() to operate on a ban spec.
I meant that That forced me to look at the rest of the ban code and found another candidate for Anyway, this command does not touch the ban list, it may only mark bans as completed. |
I get why specifying the precise ban as arguments makes sense in general, but I think we should sugar the CLI to make it easy to cancel the most recently added bans, something like: ban.cancel -4 to cancel the four most recently added bans ? |
Notes from bugwash which Dridi could not attend: @bsdphk was not aware that there is no stevedore notification of a cancel and we both agree that the implicit notification via the ban list cleanup and export can take a long time. I still think that we should notify stevedores of a cancel such that they can also neuter the respective ban(s) during a storage load. We both agree that a simple syntax is key to making this feature useful. phk's After bugwash, I wondered if we would also want an "emergency stop" for the ban lurker? |