test whether it is possible to consume before the producer gets an ack#97
test whether it is possible to consume before the producer gets an ack#97arov00 wants to merge 2 commits into
Conversation
spoiler: yes it's possible
| lastMessage.set(ack); | ||
| metricService.recordAckLatency(metadata.topic(), metadata.partition(), Duration.between(send, ack)); | ||
| Instant ackTime = Instant.ofEpochMilli(timeService.currentTimeMillis()); | ||
| Instant sendTime = Instant.ofEpochMilli(metadata.timestamp()); |
There was a problem hiding this comment.
metadata.timestamp is now set by broker? this would add another time source into the mix, no?
There was a problem hiding this comment.
I think metadata.timestamp is the timestamp we give during production, no?
There was a problem hiding this comment.
yes, depending on the topic configuration log.message.timestamp.type or message.timestamp.type.
If we are unlucky we inherit the LogAppendTime timestamp from broker config as a default when creating topics. We should ensure we set/validate CreateTime on the topics.
| if (fromRack.equals(config.rack()) && !ackedOffsets.contains(new AckedMessage(offset, partition))) { | ||
| Log.warnf("Consumed message %d:%d before receiving an ACK! This means that Ack latency could be higher than E2E latency!", partition, offset); | ||
| } else { | ||
| ackedOffsets.remove(new AckedMessage(offset, partition)); | ||
| } |
There was a problem hiding this comment.
here we cause a potential memory leak as we never remove the item that was missing here if it's added to the list later on. list will grow forever.
| .minimumExpectedValue(1.0) | ||
| .maximumExpectedValue(10_000.0) | ||
| .minimumExpectedValue(config.expectedMinLatency()) | ||
| .maximumExpectedValue(config.expectedMaxLatency()) |
There was a problem hiding this comment.
this makes sense. might be influencing the P9s to a certain extend. This change should survive.
2 participants
spoiler: yes it's possible
We observe that, in practice, Ack latencies are sometimes higher than E2E latencies at the same time and for the same topic partition. This was initially a surprise because intuitively one would expect E2E latencies to dominate ACK latencies (especially when everything runs on localhost).
Not sure if we should merge this. But it's interesting as an experiment. Running the synth client on this branch shows that it is possible for an E2E latency of an event to be recorded before its Ack latency is recorded. Normally, we would expect E2E latency to be an upper bound for the Ack latency, but the results show that this is not necessarily always the case:
If 99% of the time, the recorded Ack latencies are upper-bounded by the E2E latency, and 1% of the time they're not, then this 1% of recorded latencies ends up forming the 99th percentile of ack latencies and will be greater or equal to the E2E latency.