Added timeout non-zero verification - fixes #776 #781
Conversation
| while (m_listening) { | ||
| t.rearm(1.msecs, false); | ||
| t.wait(); | ||
| } |
There was a problem hiding this comment.
I guess this is left over from debugging?
There was a problem hiding this comment.
No, the sleep timer creates a new one every msec, it reaches 4500 IDs by the time the server is stopped
There was a problem hiding this comment.
Do you think that this is performance relevant? Timer creation is usually nothing more than assigning a few bytes of memory and incrementing a counter (plus some modulus calculation in the hash map).
There was a problem hiding this comment.
It's not very relevant for performance, I was trying to figure out where all those timers came from when tracing and afterwards, I realized it would have been easier to debug it if it showed up as the same ID.
There was a problem hiding this comment.
So it is in fact a leftover from debugging ;)
I think we should solve this at a lower level. This just makes the code less readable for no obvious gain (apart from making timer debugging slightly easier when using this specific Redis functionality, which should be a highly uncommon case). There could for example be a special per-task default timer that is used by sleep.
There was a problem hiding this comment.
Hmm, I had a feeling the proposed solution would be at a lower level.
|
looks good to me, @s-ludwig can you tag it after merging ? |
|
@extrawurst: Will do. |
Quick fix Removed RedisDebug version for faster travis builds Remove debug artifacts
|
Thanks @etcimon! Hopefully Travis is also reliable again now. |
Add non-zero timeout verification. Fixes #776. Also removes the RedisDebug version from the Redis integration test to sidestep Travis CI woes.
The pubsub listener was making the
bstopcall wait forever, because it was never listening in the first place: the 0 secondDurationtimeout was being triggered, rather than being considered as a "no timeout" value.