opendevreview | Daniel Bengtsson proposed openstack/oslo-specs master: Use cotyledon and futurist. https://review.opendev.org/c/openstack/oslo-specs/+/922597 | 11:07 |
---|---|---|
johnsom | sean-k-mooney So I am looking into this "Second simultaneous read" issue caused by https://review.opendev.org/c/openstack/oslo.log/+/918426. I see that a number of nova services are having the same exceptions based on OpenSearch. For example: | 15:25 |
johnsom | https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_6ca/889257/1/check/tempest-integrated-compute-enforce-scope-new-defaults/6cabc50/controller/logs/screen-n-sch.txt | 15:25 |
johnsom | I am wondering if this isn't a bigger issue that may call for a revert. | 15:26 |
johnsom | Another here: https://9aca0d17822e3e334dca-b5c65296db2329c92cd45b6c6555ad39.ssl.cf1.rackcdn.com/919665/2/experimental/neutron-tempest-plugin-api-ovn-wsgi/505e29a/controller/logs/screen-n-cond-cell1.txt | 15:26 |
johnsom | It looks like there were 580 instances of this log message in the last 24 hours. | 15:28 |
sean-k-mooney | johnsom: so nove has never depended on the debug feature | 16:22 |
sean-k-mooney | and i dont know of any useage of it intentioanly in nova | 16:22 |
johnsom | Right, it is oslo logging | 16:22 |
sean-k-mooney | right | 16:22 |
johnsom | basically, if you have two green threads, both of which use oslo logging, you are now going to get this exception. | 16:23 |
sean-k-mooney | you really should not oslo is ment to handel that internally | 16:24 |
johnsom | That mutex hack in oslo logging triggers the multi-reader | 16:24 |
sean-k-mooney | the one form swift | 16:25 |
sean-k-mooney | but that was somethign that was revised as part fo removing the debug flag | 16:25 |
johnsom | In my case, it's using oslo service and the loopingcall from olso service | 16:25 |
sean-k-mooney | right that could happen on any service that has a perodic | 16:25 |
sean-k-mooney | or whwere we use spawn direclty | 16:25 |
sean-k-mooney | so it sound liek the tow optoins we have are reneable the debug feature maskign the problem or also revert the mutex lock change that added this behaiovr | 16:26 |
sean-k-mooney | which would mean reverting https://github.com/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17 | 16:28 |
johnsom | Yeah, agree. I am not sure if there is a better mutex lock implementation that would work or not. I'm not deeply into eventlet. | 16:28 |
sean-k-mooney | well the issue is the oen we used is a file level one using a pip and that involes sharing a file descriptor | 16:29 |
johnsom | Personally, I would leave the mutex lock patch as it fixes the threads things, but leave the ignore multiple reader setting until we get oslo service off eventlet | 16:29 |
sean-k-mooney | the way i imeplent this in bash | 16:29 |
sean-k-mooney | is i use mkdir | 16:30 |
JayF | > logging._lock = logging.threading.RLock() | 16:30 |
JayF | that is likely the breaky code since eventlet upgrades locks now | 16:30 |
sean-k-mooney | if you use mkdir to creat a directory and two try to do that at the same tiem only one will successed | 16:30 |
sean-k-mooney | that is the one that got the lock | 16:30 |
JayF | I'm not 1000% sure; but reading that and knowing how new eventlet upgrades locks, it seems problematic | 16:30 |
sean-k-mooney | ya i think the RLOCK is now a c module or somethign liek that | 16:31 |
sean-k-mooney | the file level interprocess lock might work | 16:31 |
johnsom | Yeah, I grumble wondering why it's not using posix locks | 16:32 |
sean-k-mooney | again there you are not sharing a file descriptor or socket to hold the lock | 16:32 |
sean-k-mooney | ok oslo.logging can use oslo.utils | 16:32 |
sean-k-mooney | and oslo.utils i belive has a lock we can use instead | 16:32 |
johnsom | JayF The code looks like this now: https://github.com/openstack/oslo.log/blob/master/oslo_log/pipe_mutex.py | 16:33 |
sean-k-mooney | hum damb its in https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/lockutils.py | 16:33 |
johnsom | Yeah, the question is if eventlet will just mockeypatch over that too and "do bad things" | 16:33 |
sean-k-mooney | no | 16:34 |
sean-k-mooney | if it did we are screwed | 16:34 |
sean-k-mooney | https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/lockutils.py#L365 | 16:34 |
sean-k-mooney | so the synchronized decorator is all we need | 16:35 |
sean-k-mooney | that buils on fasteners.InterProcessLock | 16:36 |
sean-k-mooney | if its instasated with fair=true and external=true | 16:37 |
sean-k-mooney | its alos preseves fairness via a readerwriter lock internally | 16:37 |
sean-k-mooney | johnsom: its how we do all locking between eventsls by defualt in nova | 16:38 |
*** haleyb is now known as haleyb|out | 22:50 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!