gharris1727 opened a new pull request, #13185: URL: https://github.com/apache/kafka/pull/13185
[Jira](https://issues.apache.org/jira/browse/KAFKA-14670) This is the first part of the above ticket, applied only to SinkConnector and SourceConnector plugins. Additional PRs will cover the other plugins, as the refactor was too large to reasonably review at once. Design decisions: 1. The `IsolatedPlugin<P>` class will be a common superclass for all plugin wrappers. 2. The `IsolatedPlugin` superclass provides utility methods for subclasses to manage swapping the ThreadContextClassLoader for each call in a way that has minimal boilerplate. 3. The `Isolated*` classes are intended to only be constructed within the plugin isolation infrastructure, and will all have package-local constructors. 4. Testing runtime code that uses wrapped plugins will require mocking the wrappers, or instantiating a real Plugins class. 5. Subclasses should define public methods which match the plugin class they are wrapping without being an explicit subclass. These methods should be marked with `throws Exception` to remind callers that they may throw arbitrary exceptions. Open questions/issues: 1. The `hashCode`, `equals`, and `toString` methods are isolated, but do not have `throws Exception` as the Object class does not have these throws clauses. That means that calling code may not be forced to handle exceptions from these methods. We could change the methodNames, but the base Object methods would still be callable and would have a default implementation. 2. The wrapper method signatures throw `Exception` and not `Throwable`. The distinction being that `Exception`s are considered by the Java Language to be reasonable to catch in an application, and `Throwable`s were not. I wasn't sure whether the Connect runtime should be forced to handle errors like OutOfMemoryError, LinkageError, etc, or just let them propagate and kill the calling thread. 3. These wrappers do not enforce that the methods are not called on the herder thread, because I didn't come up with an elegant way to do so. 4. I did a first-pass at propagating and handling the exceptions thrown by the connector classes, but I don't know if they are reasonable. Now that the exceptions are checked, the code enforces that exceptions are handled, but it is still up to us to determine the proper way to handle the exceptions. 5. This PR does not remove existing loaderSwap calls that are currently ensuring isolation. Those can be moved/removed after all of this refactor lands, as it may still be necessary for the other plugins. ### Committer Checklist (excluded from commit message) - [ ] Verify design and implementation - [ ] Verify test coverage and CI build status - [ ] Verify documentation (including upgrade notes) -- 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
