reactor Timer ordering

[mmsolutions]

Hi,

Mostly my script is hanging when working on blocking map immediately after Hazlecast client created.
Below stack trace:

ERROR:Reactor:Error in Reactor Thread
Traceback (most recent call last):
File "C:\Tools\Python36\lib\site-packages\hazelcast\reactor.py", line 38, in _loop
self._check_timers()
File "C:\Tools\Python36\lib\site-packages\hazelcast\reactor.py", line 60, in _check_timers
self._timers.get_nowait()
File "C:\Tools\Python36\lib\queue.py", line 192, in get_nowait
return self.get(block=False)
File "C:\Tools\Python36\lib\queue.py", line 174, in get
item = self._get()
File "C:\Tools\Python36\lib\queue.py", line 230, in _get
return heappop(self.queue)
TypeError: '<' not supported between instances of 'Timer' and 'Timer'

When using pause time.sleep(1) before map consuming script works perfectly.

I think that the reason is reactor initialization, that stores Timers in queue.PriorityQueue() but Timer class is not comparable.
So I have added total ordering from functools and it works without problems.

Regards,
MM

[mdumandag]

Hi @mmsolutions
Your reasoning seems correct to me.

Elements are stored in the form of (timer.end, Timer) on the priority queue. On Windows time.time() has a bad resolution compared to UNIX systems. So, probably time.time() returns the same number in consecutive calls and priority queue is filled with elements which has same first item. Then, when first items are identical, priority queue comparison for elements falls back to Timer's itself which is not comparable.

I checked the implementation of priority queue. It only compares elements by <. So, it is enough to just implement __lt__ for this case.

Can you update your PR to add just the __lt__ method? After that, I can start the tests and merge your PR.

Thanks for your contribution

[mmsolutions]

Of course, done.

[mdumandag]

verify

[sancar]

verify

[devOpsHazelcast]

Python 3.7 Tests PASSed.

[devOpsHazelcast]

Python 2.7 Tests PASSed.

[cangencer]

I think it would be better to implement gt and eq as well, as you shouldn't rely on the fact that priority queue only checks for less than.

[mmsolutions]

Yep, that was the first commit to make Timer comparable in a full manner.
@mdumandag what do you think? Do you want to revert second one?

[mdumandag]

Okay, let's make it fully comparable. @mmsolutions Can you revert the second commit ?

[mmsolutions]

Of course @mdumandag, done.

[sancar]

verify

[devOpsHazelcast]

Python 2.7 Tests PASSed.

[devOpsHazelcast]

Python 3.7 Tests PASSed.