Skip to content

Commit e964019

Browse files
committedAug 22, 2017
Optionally use a ticker instead of a timer to detect timeouts
1 parent 7bbb175 commit e964019

File tree

3 files changed

+37
-51
lines changed

3 files changed

+37
-51
lines changed
 

‎config.go

+9-13
Original file line numberDiff line numberDiff line change
@@ -197,22 +197,20 @@ type Config struct {
197197
// (MaxProcessingTime * ChanneBufferSize). Defaults to 100ms.
198198
MaxProcessingTime time.Duration
199199

200-
// The time interval between ticks of the fast checker. A value of 0
201-
// turns off the fast checker.
202-
// If this is set to a non-zero value, then there will be periodic
203-
// checks to see if messages have been written to the Messages channel.
204-
// If a message has not been written to the Messages channel since the
205-
// last tick of the fast checker, then the timer will be set.
200+
// Whether or not to use the fast checker. The fast checker uses a
201+
// ticker instead of a timer to implement the timeout functionality in
202+
// (*partitionConsumer).responseFeeder.
203+
// If a message is not written to the Messages channel between two ticks
204+
// of the fast checker then a timeout is detected.
206205
// Using the fast checker should typically result in many fewer calls to
207206
// Timer functions resulting in a significant performance improvement if
208207
// many messages are being sent and timeouts are infrequent.
209208
// The disadvantage of using the fast checker is that timeouts will be
210209
// less accurate. That is, the effective timeout could be between
211-
// `MaxProcessingTime` and `MaxProcessingTime + FastCheckerInterval`.
212-
// For example, if `MaxProcessingTime` is 100ms and
213-
// `FastCheckerInterval` is 10ms, then a delay of 108ms between two
210+
// `MaxProcessingTime` and `2 * MaxProcessingTime`. For example, if
211+
// `MaxProcessingTime` is 100ms then a delay of 180ms between two
214212
// messages being sent may not be recognized as a timeout.
215-
FastCheckerInterval time.Duration
213+
UseFastChecker bool
216214

217215
// Return specifies what channels will be populated. If they are set to true,
218216
// you must read from them to prevent deadlock.
@@ -294,7 +292,7 @@ func NewConfig() *Config {
294292
c.Consumer.Retry.Backoff = 2 * time.Second
295293
c.Consumer.MaxWaitTime = 250 * time.Millisecond
296294
c.Consumer.MaxProcessingTime = 100 * time.Millisecond
297-
c.Consumer.FastCheckerInterval = 0
295+
c.Consumer.UseFastChecker = false
298296
c.Consumer.Return.Errors = false
299297
c.Consumer.Offsets.CommitInterval = 1 * time.Second
300298
c.Consumer.Offsets.Initial = OffsetNewest
@@ -420,8 +418,6 @@ func (c *Config) Validate() error {
420418
return ConfigurationError("Consumer.MaxWaitTime must be >= 1ms")
421419
case c.Consumer.MaxProcessingTime <= 0:
422420
return ConfigurationError("Consumer.MaxProcessingTime must be > 0")
423-
case c.Consumer.FastCheckerInterval < 0:
424-
return ConfigurationError("Consumer.FastCheckerInterval must be >= 0")
425421
case c.Consumer.Retry.Backoff < 0:
426422
return ConfigurationError("Consumer.Retry.Backoff must be >= 0")
427423
case c.Consumer.Offsets.CommitInterval <= 0:

‎consumer.go

+26-36
Original file line numberDiff line numberDiff line change
@@ -444,61 +444,51 @@ func (child *partitionConsumer) responseFeeder() {
444444
// Initialize timer without a pending send on its channel
445445
expiryTimer := time.NewTimer(0)
446446
<-expiryTimer.C
447-
expiryTimerSet := false
447+
expireTimedOut := true
448448

449-
var fastCheckerChan <-chan (time.Time)
450-
if child.conf.Consumer.FastCheckerInterval > 0 {
451-
fastChecker := time.NewTicker(child.conf.Consumer.FastCheckerInterval)
449+
var timerChan <-chan (time.Time)
450+
if child.conf.Consumer.UseFastChecker {
451+
fastChecker := time.NewTicker(child.conf.Consumer.MaxProcessingTime)
452452
defer fastChecker.Stop()
453-
fastCheckerChan = fastChecker.C
453+
timerChan = fastChecker.C
454+
} else {
455+
timerChan = expiryTimer.C
454456
}
455457

456458
feederLoop:
457459
for response := range child.feeder {
458460
msgs, child.responseResult = child.parseResponse(response)
459461

460462
for i, msg := range msgs {
461-
if child.conf.Consumer.FastCheckerInterval <= 0 {
462-
expiryTimerSet = true
463+
if !child.conf.Consumer.UseFastChecker {
464+
if !expiryTimer.Stop() && !expireTimedOut {
465+
// expiryTimer was expired; clear out the waiting msg
466+
<-expiryTimer.C
467+
}
463468
expiryTimer.Reset(child.conf.Consumer.MaxProcessingTime)
469+
expireTimedOut = false
464470
}
465471

466472
messageSelect:
467473
select {
468474
case child.messages <- msg:
469475
msgSent = true
470-
if expiryTimerSet {
471-
// The timer was set and a message was sent, stop the
472-
// timer and resume using the fast checker
473-
if !expiryTimer.Stop() {
474-
<-expiryTimer.C
476+
case <-timerChan:
477+
if !child.conf.Consumer.UseFastChecker || !msgSent {
478+
expireTimedOut = true
479+
child.responseResult = errTimedOut
480+
child.broker.acks.Done()
481+
for _, msg = range msgs[i:] {
482+
child.messages <- msg
475483
}
476-
expiryTimerSet = false
477-
}
478-
// Periodically check if messages have been sent
479-
case <-fastCheckerChan:
480-
if msgSent {
484+
child.broker.input <- child
485+
continue feederLoop
486+
} else {
487+
// current message has not been sent, return to select
488+
// statement
481489
msgSent = false
482-
} else if !expiryTimerSet {
483-
// No messages have been sent since the last tick,
484-
// start the timer
485-
expiryTimerSet = true
486-
// If the fast checker is being used, then at least
487-
// the time between two fast checker ticks has already
488-
// passed since the last message was sent.
489-
expiryTimer.Reset(child.conf.Consumer.MaxProcessingTime - child.conf.Consumer.FastCheckerInterval)
490-
}
491-
// message has not been sent, return to select statement
492-
goto messageSelect
493-
case <-expiryTimer.C:
494-
expiryTimerSet = false
495-
child.responseResult = errTimedOut
496-
child.broker.acks.Done()
497-
for _, msg = range msgs[i:] {
498-
child.messages <- msg
490+
goto messageSelect
499491
}
500-
child.broker.input <- child
501-
continue feederLoop
502492
}
503493
}
504494

‎consumer_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ func TestConsumerFastCheckerOff(t *testing.T) {
822822

823823
config := NewConfig()
824824
config.ChannelBufferSize = 0
825-
config.Consumer.FastCheckerInterval = 0
825+
config.Consumer.UseFastChecker = false
826826
config.Consumer.MaxProcessingTime = 10 * time.Millisecond
827827
master, err := NewConsumer([]string{broker0.Addr()}, config)
828828
if err != nil {
@@ -865,7 +865,7 @@ func TestConsumerFastCheckerOn(t *testing.T) {
865865

866866
config := NewConfig()
867867
config.ChannelBufferSize = 0
868-
config.Consumer.FastCheckerInterval = 1 * time.Millisecond
868+
config.Consumer.UseFastChecker = true
869869
config.Consumer.MaxProcessingTime = 10 * time.Millisecond
870870
master, err := NewConsumer([]string{broker0.Addr()}, config)
871871
if err != nil {

0 commit comments

Comments
 (0)