[GitHub] [kafka] cmccabe commented on a change in pull request #11054: KAFKA-13090: Improve kraft snapshot integration test
cmccabe commented on a change in pull request #11054:
URL: https://github.com/apache/kafka/pull/11054#discussion_r670005690
##
File path: raft/src/test/java/org/apache/kafka/raft/MockLog.java
##
@@ -489,6 +489,11 @@ public void initializeLeaderEpoch(int epoch) {
return Optional.ofNullable(snapshots.get(snapshotId));
}
+@Override
+public Optional latestSnapshot() {
+return latestSnapshotId().flatMap(this::readSnapshot);
Review comment:
I didn't realize they implemented flatMap on Java's Optional,
interesting.
--
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
[GitHub] [kafka] cmccabe commented on a change in pull request #11054: KAFKA-13090: Improve kraft snapshot integration test
cmccabe commented on a change in pull request #11054:
URL: https://github.com/apache/kafka/pull/11054#discussion_r670005192
##
File path: raft/src/main/java/org/apache/kafka/snapshot/SnapshotReader.java
##
@@ -84,7 +84,7 @@ public int lastContainedLogEpoch() {
*/
public long lastContainedLogTimestamp() {
if (!lastContainedLogTimestamp.isPresent()) {
-// nextBatch is expected to be empty
+// nextBatch is expected to be equal to Optional.empty() so just
replace it
Review comment:
Can we check this and throw an exception if it's not true? I don't think
this method is called often so performance should not be an issue.
--
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
[GitHub] [kafka] cmccabe commented on a change in pull request #11054: KAFKA-13090: Improve kraft snapshot integration test
cmccabe commented on a change in pull request #11054:
URL: https://github.com/apache/kafka/pull/11054#discussion_r670004898
##
File path: raft/src/main/java/org/apache/kafka/raft/ReplicatedLog.java
##
@@ -280,11 +280,19 @@ default long truncateToEndOffset(OffsetAndEpoch
endOffset) {
Optional readSnapshot(OffsetAndEpoch snapshotId);
/**
- * Returns the latest snapshot id if one exists.
+ * Returns the latest readable snapshot if one exists.
*
- * @return an Optional snapshot id of the latest snashot if one exists,
otherwise returns an
- * empty Optional
+ * @return an Optional with the latest readable snapshot, if one exists,
otherwise
+ * returns an empty Optional
*/
+Optional latestSnapshot();
Review comment:
would "openLatestSnapshot" be a better name, given that the snapshot
reader will need to be closed later?
--
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
