jsancio commented on a change in pull request #10909:
URL: https://github.com/apache/kafka/pull/10909#discussion_r680388871
##########
File path:
raft/src/main/java/org/apache/kafka/raft/internals/BatchAccumulator.java
##########
@@ -113,22 +116,23 @@ public Long append(int epoch, List<T> records) {
* same underlying record batch so that either all of the records become
committed or none of
* them do.
*
- * @param epoch the expected leader epoch. If this does not match, then
{@link Long#MAX_VALUE}
- * will be returned as an offset which cannot become committed
+ * @param epoch the expected leader epoch. If this does not match, then
{@link NotLeaderException}
+ * will be thrown
* @param records the list of records to include in a batch
- * @return the expected offset of the last record; {@link Long#MAX_VALUE}
if the epoch does not
- * match; null if no memory could be allocated for the batch at
this time
+ * @return the expected offset of the last record
* @throws RecordBatchTooLargeException if the size of the records is
greater than the maximum
* batch size; if this exception is throw none of the elements in
records were
* committed
+ * @throws NotLeaderException if the epoch doesn't match the leader epoch
+ * @throws BufferAllocationException if we failed to allocate memory for
the records
*/
- public Long appendAtomic(int epoch, List<T> records) {
+ public long appendAtomic(int epoch, List<T> records) {
return append(epoch, records, true);
}
- private Long append(int epoch, List<T> records, boolean isAtomic) {
+ private long append(int epoch, List<T> records, boolean isAtomic) {
if (epoch != this.epoch) {
- return Long.MAX_VALUE;
+ throw new NotLeaderException("Append failed because the epoch
doesn't match");
Review comment:
@dengziming @hachikuji What do you both think about making this
consistent with `RaftClient::resign`? `RaftClient::resign` throws
`IllegalArgumentException` when the passed epoch is greater than the current
epoch. With this change `KafkaRaftClient::schedule*Append` throws
`NotLeaderException` when the passed epoch is greater than the current epoch.
Maybe something like:
```
if (epoch > this.epoch) {
throw new IllegalArgumentExcpetion("...");
} else if (epoch < this.epoch) {
throw new NotLeaderException("...");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
