KAFKA-14466: Move ClassloaderAwareRemoteStorageManager to storage module #13013
Conversation
|
@junrao Please review when you get a chance. |
...in/java/org/apache/kafka/server/log/remote/storage/ClassLoaderAwareRemoteStorageManager.java
Outdated
Show resolved
Hide resolved
…and exception type as suggested in the review.
|
Thanks @ijuma for your review. Addressed them with the latest commit. |
| * @param <E> Exception type to be thrown | ||
| */ | ||
| @FunctionalInterface | ||
| public interface ClassLoaderAction<T, E extends Exception> { |
There was a problem hiding this comment.
I hadn't realised this is used by multiple classes. Since it is, maybe we can move this to the internals package and call it simply StorageAction or something like that. I can then use it within one of the index classes too.
There was a problem hiding this comment.
I set the name as RemoteStorageAction as it was meant for any remote storage actions. I though you wanted this to be classloader specific in your earlier comment. I am fine with renaming it to StorageAction so that it can be reused for any storage specific operations.
There was a problem hiding this comment.
Yeah, my original suggestion assumed it would be used within one class. I didn't realize we had multiple "ClassLoaderAware" classes. Once it's a top level interface, it seems better to keep it general. Thanks.
| package org.apache.kafka.server.log.remote.storage; | ||
|
|
||
| /** | ||
| * This interface is used to execute any remote storage/metadata related operation. |
There was a problem hiding this comment.
Let's update this comment to make this general to any storage action.
…fka.server.log.internals package in storage module
|
Thanks @ijuma for your review. Addressed them with the latest commit. |
|
Thanks. I'll take a look soon. |
|
Test failures are unrelated. |
ijuma
changed the title
…ule (apache#13013) Reviewers: Ismael Juma <ismael@juma.me.uk>
Committer Checklist (excluded from commit message)