nsq.Reader.disabled() no longer effective #117
Conversation
|
yep. we probably want some periodic process that "watches" the return value of |
|
finally taking a pass at this |
jehiah
force-pushed
the
nsq_reader_117
branch
2 times, most recently
from
1820e4a
to
c78113e
Compare
|
RFR @mreiferson (i'll confirm once i've validated this change set). I successfully moved the travis builds for this over to container based infra =) |
| @@ -0,0 +1,25 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Typically test.sh doesn't have side effects like installing pkgs and such. I'm fine if you want to organize code into a script for travis, but let's not call it test.sh?
There was a problem hiding this comment.
fair point. updating to travis_test.sh
|
These changes look fine but it doesn't look like it completely addresses the issue in the OP - I'm assuming that in your actual reader you're subclassing |
|
How i'm attempting to use this is here. which should expose enough to make disabling possible. I'm not 100% sure it'll work yet, but i think it will =) I propose we add a check in disabled() such that it logs a warning if used against a newer nsqd where it's inefective. We can then remove it completely at the appropriate point when we drop support for older nsqd versions. Sound good? |
OK, sounds reasonable. |
|
@mreiferson previous comments addressed. Ready for 🔨 ? |
| """ | ||
| Called as part of RDY handling to identify whether this Reader has been disabled | ||
|
|
||
| This is useful to subclass and override to examine a file on disk or a key in cache | ||
| to identify if this reader should pause execution (during a deploy, etc.). | ||
|
|
||
| Note: deprecated. Use change_max_in_flight(0) |
There was a problem hiding this comment.
we ended up calling the method set_max_in_flight
remove usage of sudo to utilize travis container infrastructure for faster integration tests
|
comments fixed and 🔨'd |
|
LGTM |
nsq.Reader.disabled() no longer effective
Due to the "don't decrement rdy" change to nsqd behavior (nsqio/nsq#404), the current effect of
disabled()returningTrueinnsq.Reader, of not sending any ready count (never calling_send_rdy()), fails to reduce the ready count to zero on the nsqd side.