You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/07/10 08:30:15 UTC

[GitHub] [kafka] zhaohaidao opened a new pull request #11015: KAFKA-12333:KafkaMetadataLog and MockLog should validate that appended epochs are monotonically

zhaohaidao opened a new pull request #11015:
URL: https://github.com/apache/kafka/pull/11015


   The original issue said both the MockLog and KafkaMetadataLog should only allow appendAsLeader and appendAsFollower with monotonically increasing epochs.
   However, interface method appendAsFollower seems not need epoch validation. So I'm not sure if which is correct. 
   @jsancio Could you please take a look and give some advice.
   
   `
   
       /**
        * Append a set of records that were replicated from the leader. The main
        * difference from appendAsLeader is that we do not need to assign the epoch
        * or do additional validation.
        *
        * @return the metadata information of the appended batch
        * @throws IllegalArgumentException if the record set is empty
        * @throws RuntimeException if the batch base offset doesn't match the log end offset
        */
       LogAppendInfo appendAsFollower(Records records);
   `
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] abbccdda commented on a change in pull request #11015: KAFKA-12333:KafkaMetadataLog and MockLog should validate that appended epochs are monotonically

Posted by GitBox <gi...@apache.org>.
abbccdda commented on a change in pull request #11015:
URL: https://github.com/apache/kafka/pull/11015#discussion_r756498765



##########
File path: core/src/main/scala/kafka/raft/KafkaMetadataLog.scala
##########
@@ -76,6 +76,9 @@ final class KafkaMetadataLog private (
     if (records.sizeInBytes == 0)
       throw new IllegalArgumentException("Attempt to append an empty record set")
 
+    if (epoch < lastFetchedEpoch())

Review comment:
       Do we need sth even more strict such that the diff between append epoch and last epoch are within 1? In what case will epoch + 2 be valid?




-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] zhaohaidao commented on pull request #11015: KAFKA-12333:KafkaMetadataLog and MockLog should validate that appended epochs are monotonically

Posted by GitBox <gi...@apache.org>.
zhaohaidao commented on pull request #11015:
URL: https://github.com/apache/kafka/pull/11015#issuecomment-886144844


   > The original issue said both the MockLog and KafkaMetadataLog should only allow appendAsLeader and appendAsFollower with monotonically increasing epochs.
   > However, It seems no need to validate epoch for appendAsFollower as far as I understand through comments. So I'm not sure which one is correct.
   > @jsancio Could you please take a look and give some advice.
   > 
   > ```
   > /**
   >  * Append a set of records that were replicated from the leader. The main
   >  * difference from appendAsLeader is that we do not need to assign the epoch
   >  * or do additional validation.
   >  *
   >  * @return the metadata information of the appended batch
   >  * @throws IllegalArgumentException if the record set is empty
   >  * @throws RuntimeException if the batch base offset doesn't match the log end offset
   >  */
   > LogAppendInfo appendAsFollower(Records records);
   > ```
   > 
   > ### Committer Checklist (excluded from commit message)
   > * [ ]  Verify design and implementation
   > * [ ]  Verify test coverage and CI build status
   > * [ ]  Verify documentation (including upgrade notes)
   
   @jsancio Could you please take a look and give some advice.


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org