Skip to content

Commit

Permalink
Fix race condition in message tracker
Browse files Browse the repository at this point in the history
  • Loading branch information
weeco committed Nov 4, 2021
1 parent a082b4e commit ffa4211
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 4 deletions.
5 changes: 3 additions & 2 deletions e2e/endtoend_message.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ type EndToEndMessage struct {
Timestamp int64 `json:"createdUtcNs"` // when the message was created, unix nanoseconds

// The following properties are only used within the message tracker
partition int
state int
partition int
state int
produceLatency float64
}

func (m *EndToEndMessage) creationTime() time.Time {
Expand Down
7 changes: 6 additions & 1 deletion e2e/message_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ func (t *messageTracker) updateItemIfExists(msg *EndToEndMessage) error {
return ttlcache.ErrNotFound
}

t.cache.SetWithTTL(msg.MessageID, msg, remainingTTL)
err = t.cache.SetWithTTL(msg.MessageID, msg, remainingTTL)
if err != nil {
panic(err)
}

return nil
}

Expand Down Expand Up @@ -129,5 +133,6 @@ func (t *messageTracker) onMessageExpired(_ string, reason ttlcache.EvictionReas
zap.Int("partition", msg.partition),
zap.String("message_id", msg.MessageID),
zap.Bool("successfully_produced", msg.state == EndToEndMessageStateProducedSuccessfully),
zap.Float64("produce_latency_seconds", msg.produceLatency),
)
}
7 changes: 6 additions & 1 deletion e2e/producer.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,12 @@ func (s *Service) produceMessage(ctx context.Context, partition int) {
// We need to use updateItemIfExists() because it's possible that the message has already been consumed
// before we have received the message here (because we were awaiting the produce ack).
msg.state = EndToEndMessageStateProducedSuccessfully
s.messageTracker.updateItemIfExists(msg)
msg.produceLatency = ackDuration.Seconds()

// TODO: Enable again as soon as https://github.com/ReneKroon/ttlcache/issues/60 is fixed
// Because we cannot update cache items in an atomic fashion we currently can't use this method
// as this would cause a race condition which ends up in records being reported as lost/expired.
// s.messageTracker.updateItemIfExists(msg)
}

s.produceLatency.WithLabelValues(pID).Observe(ackDuration.Seconds())
Expand Down

0 comments on commit ffa4211

Please sign in to comment.