[GitHub] [geode] kohlmu-pivotal commented on a change in pull request #7456: make ResourceManager's interaction with JVM customizable with the HeapUsageProvider SPI

[GitBox]

kohlmu-pivotal commented on a change in pull request #7456:
URL: https://github.com/apache/geode/pull/7456#discussion_r833809142



##########
File path: 
geode-core/src/main/java/org/apache/geode/internal/cache/control/HeapMemoryMonitor.java
##########
@@ -88,30 +90,52 @@
   @MutableForTesting
   private static long testBytesUsedForThresholdSet = -1;
 
-  @Immutable
-  private static final ServiceLoader<HeapUsageProvider> heapUsageMonitorLoader 
=
-      ServiceLoader.load(HeapUsageProvider.class);
-
   HeapMemoryMonitor(final InternalResourceManager resourceManager, final 
InternalCache cache,
       final ResourceManagerStats stats) {
+    this(resourceManager, cache, stats, createHeapUsageProvider());
+  }
+
+  @VisibleForTesting
+  HeapMemoryMonitor(final InternalResourceManager resourceManager, final 
InternalCache cache,
+      final ResourceManagerStats stats, final HeapUsageProvider 
heapUsageProvider) {
     this.resourceManager = resourceManager;
     resourceAdvisor = (ResourceAdvisor) cache.getDistributionAdvisor();
     this.cache = cache;
     this.stats = stats;
-    heapUsageMonitor = findHeapUsageMonitor();
-    thresholds = new MemoryThresholds(heapUsageMonitor.getMaxMemory());
+    this.heapUsageProvider = heapUsageProvider;
+    thresholds = new MemoryThresholds(heapUsageProvider.getMaxMemory());
     mostRecentEvent = new MemoryEvent(ResourceType.HEAP_MEMORY,
         MemoryState.DISABLED, MemoryState.DISABLED, null, 0L, true, 
thresholds);
   }
 
-  private static HeapUsageProvider findHeapUsageMonitor() {
-    for (HeapUsageProvider heapUsageMonitor : heapUsageMonitorLoader) {
-      // return the first one defined as a service
-      return heapUsageMonitor;
+  private static HeapUsageProvider createHeapUsageProvider() {
+    Optional<String> propertyValue = 
getProductStringProperty(HEAP_USAGE_PROVIDER_CLASS_NAME);

Review comment:
       Is there a way that we can add some context to the provided class name? 
`geode.classname` provides no context to the user. 
`geode.resources.memory.provider` is maybe more contextual and useful to a user.




-- 
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: notifications-unsubscr...@geode.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org