showuon commented on code in PR #14363:
URL: https://github.com/apache/kafka/pull/14363#discussion_r1320955941
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteIndexCache.java:
##########
@@ -382,6 +385,8 @@ private RemoteIndexCache.Entry
createCacheEntry(RemoteLogSegmentMetadata remoteL
TransactionIndex txnIndex = loadIndexFile(txnIndexFile,
remoteLogSegmentMetadata, rlsMetadata -> {
try {
return remoteStorageManager.fetchIndex(rlsMetadata,
IndexType.TRANSACTION);
+ } catch (RemoteResourceNotFoundException e) {
+ return null;
Review Comment:
We'd better to add some comment here to explain why we treat txnIndex
differently. Ex:
`Don't throw exception since the transaction index may not exist because of
no transactional records.`
##########
storage/api/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteStorageManager.java:
##########
@@ -123,7 +123,7 @@ InputStream fetchLogSegment(RemoteLogSegmentMetadata
remoteLogSegmentMetadata,
* Returns the index for the respective log segment of {@link
RemoteLogSegmentMetadata}.
* <p>
* Note: The transaction index may not exist because of no transactional
records.
Review Comment:
Good to me, too.
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteIndexCache.java:
##########
@@ -451,7 +456,7 @@ public static class Entry implements AutoCloseable {
private final OffsetIndex offsetIndex;
private final TimeIndex timeIndex;
- private final TransactionIndex txnIndex;
+ private final Optional<TransactionIndex> txnIndexOpt;
Review Comment:
We should make it clear again why we treat txnIndex differently. Please add
a comment. Thanks.
--
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
