-
Notifications
You must be signed in to change notification settings - Fork 15.1k
KAFKA-8729, pt 2: Add error_records and error_message to PartitionResponse #7150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9bcf5ed
abf342c
a42ef2f
e0725e4
460a3b4
61285e4
6288ad0
501a824
4f03549
ccb559f
3b8ece1
3535b69
4b830e5
d34f1b7
74ae763
18deeb4
1de5dc0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
|
|
||
| import java.nio.ByteBuffer; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
@@ -53,28 +54,37 @@ public class ProduceResponse extends AbstractResponse { | |
| /** | ||
| * Possible error code: | ||
| * | ||
| * CORRUPT_MESSAGE (2) | ||
| * UNKNOWN_TOPIC_OR_PARTITION (3) | ||
| * NOT_LEADER_FOR_PARTITION (6) | ||
| * MESSAGE_TOO_LARGE (10) | ||
| * INVALID_TOPIC (17) | ||
| * RECORD_LIST_TOO_LARGE (18) | ||
| * NOT_ENOUGH_REPLICAS (19) | ||
| * NOT_ENOUGH_REPLICAS_AFTER_APPEND (20) | ||
| * INVALID_REQUIRED_ACKS (21) | ||
| * TOPIC_AUTHORIZATION_FAILED (29) | ||
| * UNSUPPORTED_FOR_MESSAGE_FORMAT (43) | ||
| * INVALID_PRODUCER_EPOCH (47) | ||
| * CLUSTER_AUTHORIZATION_FAILED (31) | ||
| * TRANSACTIONAL_ID_AUTHORIZATION_FAILED (53) | ||
| * {@link Errors#CORRUPT_MESSAGE} | ||
| * {@link Errors#UNKNOWN_TOPIC_OR_PARTITION} | ||
| * {@link Errors#NOT_LEADER_FOR_PARTITION} | ||
| * {@link Errors#MESSAGE_TOO_LARGE} | ||
| * {@link Errors#INVALID_TOPIC_EXCEPTION} | ||
| * {@link Errors#RECORD_LIST_TOO_LARGE} | ||
| * {@link Errors#NOT_ENOUGH_REPLICAS} | ||
| * {@link Errors#NOT_ENOUGH_REPLICAS_AFTER_APPEND} | ||
| * {@link Errors#INVALID_REQUIRED_ACKS} | ||
| * {@link Errors#TOPIC_AUTHORIZATION_FAILED} | ||
| * {@link Errors#UNSUPPORTED_FOR_MESSAGE_FORMAT} | ||
| * {@link Errors#INVALID_PRODUCER_EPOCH} | ||
| * {@link Errors#CLUSTER_AUTHORIZATION_FAILED} | ||
| * {@link Errors#TRANSACTIONAL_ID_AUTHORIZATION_FAILED} | ||
| * {@link Errors#INVALID_RECORD} | ||
| */ | ||
|
|
||
| private static final String BASE_OFFSET_KEY_NAME = "base_offset"; | ||
| private static final String LOG_APPEND_TIME_KEY_NAME = "log_append_time"; | ||
| private static final String LOG_START_OFFSET_KEY_NAME = "log_start_offset"; | ||
| private static final String ERROR_RECORDS_KEY_NAME = "error_records"; | ||
| private static final String RELATIVE_OFFSET_KEY_NAME = "relative_offset"; | ||
| private static final String RELATIVE_OFFSET_ERROR_MESSAGE_KEY_NAME = "relative_offset_error_message"; | ||
| private static final String ERROR_MESSAGE_KEY_NAME = "error_message"; | ||
|
|
||
| private static final Field.Int64 LOG_START_OFFSET_FIELD = new Field.Int64(LOG_START_OFFSET_KEY_NAME, | ||
| "The start offset of the log at the time this produce response was created", INVALID_OFFSET); | ||
| private static final Field.NullableStr RELATIVE_OFFSET_ERROR_MESSAGE_FIELD = new Field.NullableStr(RELATIVE_OFFSET_ERROR_MESSAGE_KEY_NAME, | ||
| "The error message of the record that caused the batch to be dropped"); | ||
| private static final Field.NullableStr ERROR_MESSAGE_FIELD = new Field.NullableStr(ERROR_MESSAGE_KEY_NAME, | ||
| "The global error message summarizing the common root cause of the records that caused the batch to be dropped"); | ||
|
|
||
| private static final Schema PRODUCE_RESPONSE_V0 = new Schema( | ||
| new Field(RESPONSES_KEY_NAME, new ArrayOf(new Schema( | ||
|
|
@@ -149,9 +159,32 @@ public class ProduceResponse extends AbstractResponse { | |
| */ | ||
| private static final Schema PRODUCE_RESPONSE_V7 = PRODUCE_RESPONSE_V6; | ||
|
|
||
| /** | ||
| * V8 adds error_records and error_message. (see KIP-467) | ||
| */ | ||
| public static final Schema PRODUCE_RESPONSE_V8 = new Schema( | ||
| new Field(RESPONSES_KEY_NAME, new ArrayOf(new Schema( | ||
| TOPIC_NAME, | ||
| new Field(PARTITION_RESPONSES_KEY_NAME, new ArrayOf(new Schema( | ||
| PARTITION_ID, | ||
| ERROR_CODE, | ||
| new Field(BASE_OFFSET_KEY_NAME, INT64), | ||
| new Field(LOG_APPEND_TIME_KEY_NAME, INT64, "The timestamp returned by broker after appending " + | ||
| "the messages. If CreateTime is used for the topic, the timestamp will be -1. " + | ||
| "If LogAppendTime is used for the topic, the timestamp will be the broker local " + | ||
| "time when the messages are appended."), | ||
| LOG_START_OFFSET_FIELD, | ||
| new Field(ERROR_RECORDS_KEY_NAME, new ArrayOf(new Schema( | ||
| new Field.Int32(RELATIVE_OFFSET_KEY_NAME, "The relative offset of the record " + | ||
| "that caused the batch to be dropped"), | ||
| RELATIVE_OFFSET_ERROR_MESSAGE_FIELD | ||
| )), "The relative offsets of records that caused the batch to be dropped"), | ||
| ERROR_MESSAGE_FIELD)))))), | ||
|
hachikuji marked this conversation as resolved.
|
||
| THROTTLE_TIME_MS); | ||
|
|
||
| public static Schema[] schemaVersions() { | ||
| return new Schema[]{PRODUCE_RESPONSE_V0, PRODUCE_RESPONSE_V1, PRODUCE_RESPONSE_V2, PRODUCE_RESPONSE_V3, | ||
| PRODUCE_RESPONSE_V4, PRODUCE_RESPONSE_V5, PRODUCE_RESPONSE_V6, PRODUCE_RESPONSE_V7}; | ||
| PRODUCE_RESPONSE_V4, PRODUCE_RESPONSE_V5, PRODUCE_RESPONSE_V6, PRODUCE_RESPONSE_V7, PRODUCE_RESPONSE_V8}; | ||
| } | ||
|
|
||
| private final Map<TopicPartition, PartitionResponse> responses; | ||
|
|
@@ -183,15 +216,28 @@ public ProduceResponse(Struct struct) { | |
| for (Object topicResponse : struct.getArray(RESPONSES_KEY_NAME)) { | ||
| Struct topicRespStruct = (Struct) topicResponse; | ||
| String topic = topicRespStruct.get(TOPIC_NAME); | ||
|
|
||
| for (Object partResponse : topicRespStruct.getArray(PARTITION_RESPONSES_KEY_NAME)) { | ||
| Struct partRespStruct = (Struct) partResponse; | ||
| int partition = partRespStruct.get(PARTITION_ID); | ||
| Errors error = Errors.forCode(partRespStruct.get(ERROR_CODE)); | ||
| long offset = partRespStruct.getLong(BASE_OFFSET_KEY_NAME); | ||
| long logAppendTime = partRespStruct.getLong(LOG_APPEND_TIME_KEY_NAME); | ||
| long logStartOffset = partRespStruct.getOrElse(LOG_START_OFFSET_FIELD, INVALID_OFFSET); | ||
|
|
||
| Map<Integer, String> errorRecords = new HashMap<>(); | ||
| if (partRespStruct.hasField(ERROR_RECORDS_KEY_NAME)) { | ||
| for (Object recordOffsetAndMessage : partRespStruct.getArray(ERROR_RECORDS_KEY_NAME)) { | ||
| Struct recordOffsetAndMessageStruct = (Struct) recordOffsetAndMessage; | ||
| Integer relativeOffset = recordOffsetAndMessageStruct.getInt(RELATIVE_OFFSET_KEY_NAME); | ||
| String relativeOffsetErrorMessage = recordOffsetAndMessageStruct.getOrElse(RELATIVE_OFFSET_ERROR_MESSAGE_FIELD, ""); | ||
| errorRecords.put(relativeOffset, relativeOffsetErrorMessage); | ||
| } | ||
| } | ||
|
|
||
| String errorMessage = partRespStruct.getOrElse(ERROR_MESSAGE_FIELD, ""); | ||
| TopicPartition tp = new TopicPartition(topic, partition); | ||
| responses.put(tp, new PartitionResponse(error, offset, logAppendTime, logStartOffset)); | ||
| responses.put(tp, new PartitionResponse(error, offset, logAppendTime, logStartOffset, errorRecords, errorMessage)); | ||
| } | ||
| } | ||
| this.throttleTimeMs = struct.getOrElse(THROTTLE_TIME_MS, DEFAULT_THROTTLE_TIME); | ||
|
|
@@ -223,6 +269,18 @@ protected Struct toStruct(short version) { | |
| if (partStruct.hasField(LOG_APPEND_TIME_KEY_NAME)) | ||
| partStruct.set(LOG_APPEND_TIME_KEY_NAME, part.logAppendTime); | ||
| partStruct.setIfExists(LOG_START_OFFSET_FIELD, part.logStartOffset); | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: we can probably do some cleanup in pr.3, e.g. line 269 above can be replaced with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cc @tuvtran
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ack :) |
||
| List<Struct> errorRecords = new ArrayList<>(); | ||
| for (Map.Entry<Integer, String> recordOffsetAndMessage : part.errorRecords.entrySet()) { | ||
| Struct recordOffsetAndMessageStruct = partStruct.instance(ERROR_RECORDS_KEY_NAME) | ||
| .set(RELATIVE_OFFSET_KEY_NAME, recordOffsetAndMessage.getKey()) | ||
| .setIfExists(RELATIVE_OFFSET_ERROR_MESSAGE_FIELD, recordOffsetAndMessage.getValue()); | ||
| errorRecords.add(recordOffsetAndMessageStruct); | ||
| } | ||
|
|
||
| partStruct.setIfExists(ERROR_RECORDS_KEY_NAME, errorRecords.toArray()); | ||
|
|
||
| partStruct.setIfExists(ERROR_MESSAGE_FIELD, part.errorMessage); | ||
| partitionArray.add(partStruct); | ||
| } | ||
| topicData.set(PARTITION_RESPONSES_KEY_NAME, partitionArray.toArray()); | ||
|
|
@@ -256,16 +314,28 @@ public static final class PartitionResponse { | |
| public long baseOffset; | ||
| public long logAppendTime; | ||
| public long logStartOffset; | ||
| public Map<Integer, String> errorRecords; | ||
| public String errorMessage; | ||
|
|
||
| public PartitionResponse(Errors error) { | ||
| this(error, INVALID_OFFSET, RecordBatch.NO_TIMESTAMP, INVALID_OFFSET); | ||
| } | ||
|
|
||
| public PartitionResponse(Errors error, long baseOffset, long logAppendTime, long logStartOffset) { | ||
| this(error, baseOffset, logAppendTime, logStartOffset, Collections.emptyMap(), null); | ||
| } | ||
|
|
||
| public PartitionResponse(Errors error, long baseOffset, long logAppendTime, long logStartOffset, Map<Integer, String> errorRecords) { | ||
| this(error, baseOffset, logAppendTime, logStartOffset, errorRecords, ""); | ||
| } | ||
|
|
||
| public PartitionResponse(Errors error, long baseOffset, long logAppendTime, long logStartOffset, Map<Integer, String> errorRecords, String errorMessage) { | ||
| this.error = error; | ||
| this.baseOffset = baseOffset; | ||
| this.logAppendTime = logAppendTime; | ||
| this.logStartOffset = logStartOffset; | ||
| this.errorRecords = errorRecords; | ||
| this.errorMessage = errorMessage; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -280,6 +350,10 @@ public String toString() { | |
| b.append(logAppendTime); | ||
| b.append(", logStartOffset: "); | ||
| b.append(logStartOffset); | ||
| b.append(", errorRecords: "); | ||
| b.append(errorRecords); | ||
| b.append(", errorMessage: "); | ||
| b.append(errorMessage); | ||
| b.append('}'); | ||
| return b.toString(); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. Isn't this an incompatible change? Does this exception not get exposed anywhere currently? Maybe it would be better to deprecate
common/record/InvalidRecordException, but still leave it around.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think
InvalidRecordExceptionwas not exposed as a public API before, and now we are moving its package as well to make it a public class.