harlowja / fasteners

A python package that provides useful locks.
Apache License 2.0
248 stars 45 forks source link

support eventlet spawn_n #97

Closed SeanMooney closed 2 years ago

SeanMooney commented 2 years ago
psarka commented 2 years ago

Thanks @SeanMooney, glad you figured out what exactly is wrong.

May I suggest, however, a slightly lighter approach?

Could we reintroduce the current_thread_functor, but leave the rest outside fasteners? Would it be acceptable for oslo.concurrency to create the functor and pass it to the ReaderWriterLock?

SeanMooney commented 2 years ago

I think that should be doable.

there are some disadvantages to this approach in that we have 2 directly call fasteners they will need to have the same modification as would any non OpenStack users that use eventlet but i suspect that the over lap of eventlet + fasteners - OpenStack is small.

the other disadvantage is that oslo need to support multiple versions of fasteners in general. so in my patch to oslo.concurrency i need detect the version of fasteners deployed or raise the min version required.

i can get the fastener version string https://github.com/harlowja/fasteners/blob/main/fasteners/version.py#L23

although its slightly unfortunet that its wrong for 0.15 https://github.com/harlowja/fasteners/blob/0.15/fasteners/version.py

based on the current code

NOT_USED = object()

class ReaderWriterLock(object): """An inter-thread readers writer lock."""

WRITER = 'w'  #: Writer owner type/string constant.
READER = 'r'  #: Reader owner type/string constant.

def __init__(self,
             condition_cls=threading.Condition,
             current_thread_functor=NOT_USED):
    """
    Args:
        condition_cls:
            Optional custom `Condition` primitive used for synchronization.
        current_thread_functor:
            Not used, will be removed in 1.0.
    """
    if current_thread_functor != NOT_USED:
        warnings.warn('current_thread_functor is deprecated (in fact not used) and will be '
                      'removed in 1.0 release.', DeprecationWarning)

    self._writer = None
    self._pending_writers = collections.deque()
    self._readers = {}
    self._cond = condition_cls()
    self._current_thread = threading.current_thread

the other way to detect this si to check if self._current_thread is the functor i passed in and or if the NOT_USED object is defined in the module since it likely makes sense to remove that in this PR. ill need to get some feedback form the oslo maintainer on there prefercne (detecting or raising min version) but we have a way forward i think so ill start working on the updated pr and gerrit review to address this via passing in a fucntor as you suggest

SeanMooney commented 2 years ago

i have proposed

https://review.opendev.org/c/openstack/project-config/+/856096 and https://review.opendev.org/c/openstack/oslo.concurrency/+/856120

to add fasternes to the repos we can depend on in our ci system and to have oslo use the public interface to pass the thread functor when available

I'm assuming that there will be a 0.17.4 release at somepoint with this change hence the -w on the patch as i want to confirm if that is correct before we proceed with this on the oslo side.

as you can likely see we have already merged https://review.opendev.org/c/openstack/oslo.concurrency/+/855714 to workaround this by setting the private _current_thread attribute in the absence of a way to set this in the init_function so that will work until this is supported by fasteners but obviously it preferable to not depend on internal field in a third party lib so once there is an official interface for this again it will be better to use that.