Owen-CH-Leung commented on code in PR #14057:
URL: https://github.com/apache/kafka/pull/14057#discussion_r1269422337
##########
storage/src/test/java/org/apache/kafka/server/log/remote/storage/LocalTieredStorageTest.java:
##########
@@ -399,20 +403,21 @@ public Verifier(final LocalTieredStorage remoteStorage,
final TopicIdPartition t
this.topicIdPartition = requireNonNull(topicIdPartition);
}
- private List<Path> expectedPaths(final RemoteLogSegmentId id) {
+ private List<Path> expectedPaths(final RemoteLogSegmentMetadata
metadata) {
final String rootPath = getStorageRootDirectory();
TopicPartition tp = topicIdPartition.topicPartition();
final String topicPartitionSubpath = format("%s-%d-%s",
tp.topic(), tp.partition(),
topicIdPartition.topicId());
- final String uuid = id.id().toString();
+ final String uuid = metadata.remoteLogSegmentId().id().toString();
+ final long startOffset = metadata.startOffset();
return Arrays.asList(
- Paths.get(rootPath, topicPartitionSubpath, uuid +
LogFileUtils.LOG_FILE_SUFFIX),
- Paths.get(rootPath, topicPartitionSubpath, uuid +
LogFileUtils.INDEX_FILE_SUFFIX),
- Paths.get(rootPath, topicPartitionSubpath, uuid +
LogFileUtils.TIME_INDEX_FILE_SUFFIX),
- Paths.get(rootPath, topicPartitionSubpath, uuid +
LogFileUtils.TXN_INDEX_FILE_SUFFIX),
- Paths.get(rootPath, topicPartitionSubpath, uuid +
LEADER_EPOCH_CHECKPOINT.getSuffix()),
- Paths.get(rootPath, topicPartitionSubpath, uuid +
LogFileUtils.PRODUCER_SNAPSHOT_FILE_SUFFIX)
+ Paths.get(rootPath, topicPartitionSubpath, startOffset +
"-" + uuid + LogFileUtils.LOG_FILE_SUFFIX),
Review Comment:
@divijvaidya Thanks for your feedback. I think the actual log file was named
as [offset.filetype]. Looking at the implementation of
`LogFileUtils#logFile(File dir, long offset)`, I don't think it will allow us
to insert a uuid in the middle as part of the filename.
If we are to keep the `[offset-uuid.filetype]` pattern, instead of using
`LogFileUtils#logFile(File dir, long offset)`, maybe we should make
`LogFileUtils#filenamePrefixFromOffset(long offset)` as a public method so that
we can construct a real offset using this method. What do you think ?
FYI, the method to create these offloaded files is
`RemoteLogSegmentFileset#openFileset(final File storageDir, final
RemoteLogSegmentId id)` . Currently my PR has changed this method to accept
`RemoteLogSegmentMetadata` instead of `RemoteLogSegmentId` , get offset from
metadata, and prepend it to the filename. (So yes, it's not close to the actual
log file implementation, as the offset was just "0" without formatting, instead
of "0000000")
--
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
