-
Notifications
You must be signed in to change notification settings - Fork 14.4k
MINOR: Remove unused code from storage classes #19853
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
@chia7712 PTAL |
@@ -42,12 +42,6 @@ public void handleRemoteLogMetadata(RemoteLogMetadata remoteLogMetadata) { | |||
|
|||
protected abstract void handleRemotePartitionDeleteMetadata(RemotePartitionDeleteMetadata remotePartitionDeleteMetadata); | |||
|
|||
public void syncLogMetadataSnapshot(TopicIdPartition topicIdPartition, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
KAFKA-16454 was filed to snapshot the __remote_log_metadata topic for faster restarts, so the syncLogMetadataSnapshot
and RemoteLogSegmentMetadataSnapshot.java
was not removed.
is it ok to remove it?
The unused syncLogMetadataSnapshot
method can be removed.
return config.getString(REMOTE_STORAGE_MANAGER_CONFIG_PREFIX_PROP); | ||
} | ||
|
||
public String remoteLogMetadataManagerPrefix() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should not remove the "prefix" configs, they are used by the RemoteStorageManager and RemoteLogMetadataManger plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restored the prefix configs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can add comments to avoid exceptional deletion down the line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about adding @SuppressWarnings("unused")
to those unused methods or variables if we know they’re meant to be used elsewhere (or in the future)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good idea.
Added @SuppressWarnings("unused")
and comments.
return the prefix configs
address review comments
Remove unused code from storage classes.