ctubbsii commented on a change in pull request #1572: Refactor use of
ServerConfigurationFactory
URL: https://github.com/apache/accumulo/pull/1572#discussion_r399459647
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/tabletserver/MemoryManager.java
##########
@@ -32,8 +33,22 @@
/**
* Initialize the memory manager.
+ *
+ * @deprecated since 2.1 use {@link #init(ServerContext)}
+ */
+ @Deprecated
+ default void init(ServerConfiguration conf) {
+ throw new IllegalStateException("Deprecated method scheduled for removal
in 3.0.0");
+ }
+
+ /**
+ * Initialize the memory manager.
+ *
+ * @param context
+ * ServerContext passed from the Tablet server
+ * @since 2.1
*/
- void init(ServerConfiguration conf);
+ void init(ServerContext context);
Review comment:
My only concern with this is that context is not public API any more than
ServerConfiguration was. The object has already changed from a
ServerConfiguration object to a ServerConfigurationFactory object (which
implemented ServerConfiguration, just to make this API happy), and now
ServerContext.
While it's unlikely that too many people have implemented their own
MemoryManager, it is configurable, and this churn in the API isn't good for
implementers. I think it would be cool if we could follow up on this PR with
one that promotes this interface to SPI, and uses a stable
MemoryManagerEnvironment object to inject here in place of the internal
ServerContext.
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
