[swift] Can I use oslo.log / oslo.config in swift? Can we make Swift behave like the rest of OpenStack?
Hi,
Because we had some issues with a bad rebalance in production (we didn't loose any data, but some drives went full...), I attempted to implement the Swift mitigation to prevent disk full scenarios. I found that what upstream recommends [1] is a script that's written in Python 2, and that wouldn't work at all: it's not only very old, it's also a very partial implementation.
So I wrote this: https://review.opendev.org/c/openstack/swift/+/907523
But then, I was told (thanks for the review!) that I shouldn't be using oslo.config and/or oslo.log, as other parts of Swift don't do that. I do understand that the rest of Swift is like this, and therefore, the review comment is probably right.
However...
Back in the days, I would have understood why things would have been written that way: the Oslo libs were young, and not available everywhere, and it was kind of a good idea not to use them. But now, fast forward in 2024, I found that Swift behaving differently from the rest of OpenStack is more a problem than a feature. It's also very frustrating to use the old way with the config_parser, which can't auto-generate the config file and so on.
So I am wondering: - is it still the case, that I shouldn't use oslo.config / oslo.log when adding a patch in Swift? - can we revisit this, and convert swift to oslo.config generated config files, and PBR shell-script entry points instead of binary simply dropped into the toplevel bin folder?
Can we make swift smarter and behave like the rest of OpenStack? :)
Cheers,
Thomas Goirand (zigo)
[1] https://docs.openstack.org/swift/latest/admin_guide.html#preventing-disk-ful...
On Sun, 11 Feb 2024 21:07:42 +0100 Thomas Goirand zigo@debian.org wrote:
Back in the days, I would have understood why things would have been written that way: the Oslo libs were young, and not available everywhere, and it was kind of a good idea not to use them.
So I am wondering:
- is it still the case, that I shouldn't use oslo.config / oslo.log when
adding a patch in Swift?
- can we revisit this, and convert swift to oslo.config generated config
files, and PBR shell-script entry points instead of binary simply dropped into the toplevel bin folder?
I think Swift being different here is a technical baggage at this point. Switching over will please some, but upset the constituency of heavy users of Swift. We can try to stay compatible, of course.
At my side, the get_logger() was particularly problematic when keystone token middleware was shared, because it uses LOG and that just does not produce any output. I never had an issue with logging_monkey_patch(), but I can see that too.
The main problem is just that Oslo drags in a great many dependencies, and Swift used to be installed standalone by many operators. With container images becoming the prevalent installation method, this concern may be not as significant as it used to be. But honestly I would prefer to register your concern and leave it be. Too much other work.
BTW, I'll take a look at that drive checker review.
-- Pete
On 3/19/24 15:38, Pete Zaitcev wrote:
On Sun, 11 Feb 2024 21:07:42 +0100 Thomas Goirand zigo@debian.org wrote:
So I am wondering:
- is it still the case, that I shouldn't use oslo.config / oslo.log when
adding a patch in Swift?
- can we revisit this, and convert swift to oslo.config generated config
files, and PBR shell-script entry points instead of binary simply dropped into the toplevel bin folder?
I think Swift being different here is a technical baggage at this point. Switching over will please some, but upset the constituency of heavy users of Swift. We can try to stay compatible, of course.
At which point are we considered a "swift heavy user"? Is 6500+ HDD and 101 PB in production considered "heavy"? If such, then I'm part of this group of user, and I wont be upset... :)
I'm of the opinion that it'd be nice if we could relax the current policy, and make swift at least start logging like the other projects
BTW, I'll take a look at that drive checker review.
Thanks, though at this time, we want to rewrite it so that it uses the /etc/rsyncd.d folder, to do overrides rather than rewriting all the file. We're currently busy with production, so it will take a bit of time to get there, plus we need the puppetlabs-rsync to accept our patch to handle the &include in rsyncd.conf (that it currently doesn't support). So you may want to hold on reviewing that patch until we're fully done.
Cheers,
Thomas Goirand (zigo)
participants (2)
-
Pete Zaitcev
-
Thomas Goirand