michaeljmarshall opened a new pull request #9878:
URL: https://github.com/apache/pulsar/pull/9878


   ### Motivation
   
   In Pulsar 2.7.0, there is a class loader leak. It looks like 
https://github.com/apache/pulsar/pull/8739 fixed the leak by only loading the 
offloader classes for the directory configured in `broker.conf`. However, the 
solution in https://github.com/apache/pulsar/pull/8739 ignores the fact that an 
offload policy can override the the offloaded directory. As such, there could 
be a regression in 2.7.1 if users are providing multiple offload directories.
   
   This PR returns the functionality without reintroducing the class loader 
leak.
   
   ### Modifications
   
   Update the `PulsarService` and the `PulsarConnectorCache` classes to use a 
map from directory strings to `Offloaders`.
   
   ### Alternative Approaches
   
   The new `Map` has keys of type `String`, but we could use keys of type 
`Path` and then normalize the paths to ensure that `./offloaders` and 
`offloaders` result in a single class loader. However, it looks like the 
`normalize` method in the path class has a warning about symbolic links. As 
such, I went with the basic `String` approach, which might lead to some 
duplication of loaded classes. Below is the javadoc for `normalize`, in case 
that helps for a design decision.
   
   ```java
     /**
        * Returns a path that is this path with redundant name elements 
eliminated.
        *
        * <p> The precise definition of this method is implementation dependent 
but
        * in general it derives from this path, a path that does not contain
        * <em>redundant</em> name elements. In many file systems, the "{@code 
.}"
        * and "{@code ..}" are special names used to indicate the current 
directory
        * and parent directory. In such file systems all occurrences of "{@code 
.}"
        * are considered redundant. If a "{@code ..}" is preceded by a
        * non-"{@code ..}" name then both names are considered redundant (the
        * process to identify such names is repeated until it is no longer
        * applicable).
        *
        * <p> This method does not access the file system; the path may not 
locate
        * a file that exists. Eliminating "{@code ..}" and a preceding name 
from a
        * path may result in the path that locates a different file than the 
original
        * path. This can arise when the preceding name is a symbolic link.
        *
        * @return  the resulting path or this path if it does not contain
        *          redundant name elements; an empty path is returned if this 
path
        *          does have a root component and all name elements are 
redundant
        *
        * @see #getParent
        * @see #toRealPath
        */
       Path normalize();
   ```
   
   ### Verifying this change
   
   This change is a code cleanup without any test coverage that should be 
covered by other tests. If required, I can create some tests.
   
   ### Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): no
     - The public API: no
     - The schema: no
     - The default values of configurations: no
     - The wire protocol: no
     - The rest endpoints: no
     - The admin cli options: no
     - Anything that affects deployment: no
   
   ### Documentation
   
   None needed.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to