KAFKA-5353: baseTimestamp should always have a create timestamp #3177
Conversation
|
Review by @hachikuji |
ijuma
changed the title
ijuma
changed the title
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Thanks for the patch. We should definitely fix the inconsistent behavior. It does seem a bit unfortunate to lose the create timestamp information though if we don't have to. Alternatively, we can let base timestamp always be the create time and ensure that the relative timestamps are preserved when recompressing on the broker. Since we have the timestamp type field in the batch, we can still override the individual timestamps to use the log append time before returning to users. And perhaps in the future we will have a use case which calls for having the create time available regardless of the timestamp type. At a minimum, it may be useful for debugging. |
ijuma
force-pushed
the
set-base-sequence-for-log-append-time
branch
from
12777a5
to
d19c9eb
Compare
ijuma
changed the title
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
This makes the case where we build the records from scratch consistent with the case where update the batch header "in place". Thanks to edenhill who found the issue while testing librdkafka. The reason our tests don’t catch this is that we rely on the maxTimestamp to compute the record level timestamps if log append time is used. Author: Ismael Juma <ismael@juma.me.uk> Reviewers: Jason Gustafson <jason@confluent.io> Closes #3177 from ijuma/set-base-sequence-for-log-append-time (cherry picked from commit 647afef) Signed-off-by: Ismael Juma <ismael@juma.me.uk>
This makes the case where we build the records from scratch consistent
with the case where update the batch header "in place". Thanks to
@edenhill who found the issue while testing librdkafka.
The reason our tests don’t catch this is that we rely on the maxTimestamp
to compute the record level timestamps if log append time is used.