You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "EdColeman (via GitHub)" <gi...@apache.org> on 2023/04/11 00:00:16 UTC

[GitHub] [accumulo] EdColeman opened a new pull request, #3288: add low memory metric to abstract server

EdColeman opened a new pull request, #3288:
URL: https://github.com/apache/accumulo/pull/3288

   Adds metric to detect low memory conditions
   
    - metric added to AbstractServer so all processes inherit the reporting of the value.
    - includes minor ide checkstyle suggested fixes to tests.


-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1169385803


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    lowMemoryMetricGuage =
+        Gauge
+            .builder(METRICS_APP_PREFIX + applicationName + "." + hostname + "."

Review Comment:
   Leveraged tags in 674a9da956



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#issuecomment-1502503461

   Fixes issue https://github.com/apache/accumulo/issues/3243


-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] dlmarion commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1169907576


##########
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java:
##########
@@ -602,7 +602,7 @@ public void run() {
       LOG.error("Error initializing metrics, metrics will not be emitted.", e1);
     }
     pausedMetrics = new PausedCompactionMetrics();
-    MetricsUtil.initializeProducers(this, pausedMetrics);
+    initServerMetrics(pausedMetrics);

Review Comment:
   I think `this` was dropped accidentally. `Compactor` implements `MetricsProducer` too, so it's `registerMetrics` method is not getting called.



##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +94,16 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    processMetrics.registerMetrics(registry);
+  }
+
+  public void initServerMetrics(MetricsProducer... producers) {

Review Comment:
   I don't think this method is necessary, and it's initializing the metrics for AbstractServer differently than everything else. Since you have modified `AbstractServer` to implement `MetricsProducer`, then:
   
     1. You should have any subclasses of `AbstractServer` call `super.registerMetrics` in their respective `registerMetrics` method. For example, [Compactor.registerMetrics](https://github.com/apache/accumulo/blob/15c9f55280f4ce28a80b800f1e05128f33c85db3/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java#L170).
     2. You should pass `this` in subclasses of `AbstractServer` in the call to `MetricsUtil.initializeProducers`.



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] ddanielr commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1163082934


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    lowMemoryMetricGuage =
+        Gauge
+            .builder(METRICS_APP_PREFIX + applicationName + "." + hostname + "."

Review Comment:
   I'm not super familiar with Micrometer, but the way this is currently implemented, wouldn't
   the metrics be created in the following structure?
   `accumulo.app.app1.server1.detected.low.memory = 1|0` 
   `accumulo.app.app1.server2.detected.low.memory = 1|0`
   `accumulo.app.app2.server1.detected.low.memory = 1|0`
   
   I'm pretty sure these would all be considered unique metrics.
   
   Does Micrometer support tags or metric labels? 
   If the application name and host name were added as tags on the metric, then the metric would look like
   `accumulo.app.detected.low.memory{"application Name": "app1", "hostname": "server1"} = 1|0` 
   
   This drops the unique metric definition down to `accumulo.app.detected.low.memory` and allows sorting and filtering based on `application name` or `hostname`. 
   
   Alerting still works by firing on `accumulo.app.detected.low.memory =1` but better granularity is allowed on the visualization and alerting ends.
   



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] ctubbsii commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "ctubbsii (via GitHub)" <gi...@apache.org>.
ctubbsii commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1163411652


##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java:
##########
@@ -594,6 +601,8 @@ public interface MetricsProducer {
 
   Logger LOG = LoggerFactory.getLogger(MetricsProducer.class);
 
+  String METRICS_APP_PREFIX = "accumulo.app.";

Review Comment:
   "app" sounds like "mobile app", and sounds a bit "trendy" and less serious/scientific. Maybe "server"? Or just "accumulo."? Or "accumulo.application"?



##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    lowMemoryMetricGuage =
+        Gauge
+            .builder(METRICS_APP_PREFIX + applicationName + "." + hostname + "."
+                + METRICS_APP_LOW_MEMORY, this, this::lowMemDetected)
+            .description(
+                "reports 1 when process memory usage is above threshold, 0 when memory is okay") // optional
+            .register(registry);
+  }
+
+  private int lowMemDetected(AbstractServer abstractServer) {
+    if (abstractServer.context.getLowMemoryDetector().isRunningLowOnMemory()) {
+      return 1;
+    }
+    return 0;

Review Comment:
   ```suggestion
       return abstractServer.context.getLowMemoryDetector().isRunningLowOnMemory() ? 1 : 0;
   ```
   
   Or, this looks cleaner, because it seems to be passing the current object to itself:
   
   ```suggestion
       return context.getLowMemoryDetector().isRunningLowOnMemory() ? 1 : 0;
   ```



##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -35,21 +36,25 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public abstract class AbstractServer implements AutoCloseable, Runnable {
+import io.micrometer.core.instrument.Gauge;
+import io.micrometer.core.instrument.MeterRegistry;
+
+public abstract class AbstractServer implements AutoCloseable, MetricsProducer, Runnable {
 
   private final ServerContext context;
   protected final String applicationName;
   private final String hostname;
-  private final Logger log;
+
+  private Gauge lowMemoryMetricGuage = null;
 
   protected AbstractServer(String appName, ConfigOpts opts, String[] args) {
-    this.log = LoggerFactory.getLogger(getClass().getName());
     this.applicationName = appName;
     opts.parseArgs(appName, args);
     var siteConfig = opts.getSiteConfiguration();
     this.hostname = siteConfig.get(Property.GENERAL_PROCESS_BIND_ADDRESS);
     SecurityUtil.serverLogin(siteConfig);
     context = new ServerContext(siteConfig);
+    Logger log = LoggerFactory.getLogger(getClass().getName());

Review Comment:
   I think you can just use the class, rather than explicitly call `.getName()`.
   ```suggestion
       Logger log = LoggerFactory.getLogger(getClass());
   ```



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman merged pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman merged PR #3288:
URL: https://github.com/apache/accumulo/pull/3288


-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1169385903


##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java:
##########
@@ -594,6 +601,8 @@ public interface MetricsProducer {
 
   Logger LOG = LoggerFactory.getLogger(MetricsProducer.class);
 
+  String METRICS_APP_PREFIX = "accumulo.app.";

Review Comment:
   addressed in 674a9da956



##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -35,21 +36,25 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public abstract class AbstractServer implements AutoCloseable, Runnable {
+import io.micrometer.core.instrument.Gauge;
+import io.micrometer.core.instrument.MeterRegistry;
+
+public abstract class AbstractServer implements AutoCloseable, MetricsProducer, Runnable {
 
   private final ServerContext context;
   protected final String applicationName;
   private final String hostname;
-  private final Logger log;
+
+  private Gauge lowMemoryMetricGuage = null;
 
   protected AbstractServer(String appName, ConfigOpts opts, String[] args) {
-    this.log = LoggerFactory.getLogger(getClass().getName());
     this.applicationName = appName;
     opts.parseArgs(appName, args);
     var siteConfig = opts.getSiteConfiguration();
     this.hostname = siteConfig.get(Property.GENERAL_PROCESS_BIND_ADDRESS);
     SecurityUtil.serverLogin(siteConfig);
     context = new ServerContext(siteConfig);
+    Logger log = LoggerFactory.getLogger(getClass().getName());

Review Comment:
   fixed in 674a9da956



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1169386378


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    lowMemoryMetricGuage =
+        Gauge
+            .builder(METRICS_APP_PREFIX + applicationName + "." + hostname + "."
+                + METRICS_APP_LOW_MEMORY, this, this::lowMemDetected)
+            .description(
+                "reports 1 when process memory usage is above threshold, 0 when memory is okay") // optional
+            .register(registry);
+  }
+
+  private int lowMemDetected(AbstractServer abstractServer) {
+    if (abstractServer.context.getLowMemoryDetector().isRunningLowOnMemory()) {
+      return 1;
+    }
+    return 0;

Review Comment:
   fixed in 674a9da956 (method moved to ProcessMetrics)



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1172947619


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +94,16 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    processMetrics.registerMetrics(registry);
+  }
+
+  public void initServerMetrics(MetricsProducer... producers) {

Review Comment:
   Addressed in 49b93b349e - backed of removal of MetricsUtil for now to minimize changes.  May look into additional changes in future PRs, but this code works as expected.



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] ddanielr commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1163082934


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    lowMemoryMetricGuage =
+        Gauge
+            .builder(METRICS_APP_PREFIX + applicationName + "." + hostname + "."

Review Comment:
   I'm not super familiar with Micrometer, but the way this is currently implemented, wouldn't
   the metrics be created in the following structure?
   `accumulo.app.app1.server1.detected.low.memory = 1|0` 
   `accumulo.app.app1.server2.detected.low.memory = 1|0`
   `accumulo.app.app2.server1.detected.low.memory = 1|0`
   
   I'm pretty sure these would all be considered unique metrics.
   
   Does Micrometer support tags or metric labels? 
   If the application name and host name were added as tags on the metric, then the metric would look like
   `accumulo.app.detected.low.memory{"application Name": "app1", "hostname": server1} = 1|0` 
   
   This drops the unique metric definition down to `accumulo.app.detected.low.memory` and allows sorting and filtering based on `application name` or `hostname`. 
   
   Alerting still works by firing on `accumulo.app.detected.low.memory =1` but better granularity is allowed on the visualization and alerting ends.
   



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] dlmarion commented on pull request #3288: add low memory metric to abstract server

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#issuecomment-1517722281

   I think with these changes overall, all subclasses of AbstractServer need to call MetricsUtil.initializeProducers, passing in `this` at a minimum. I think that CompactionCoordinator is missing this.


-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] dlmarion commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1170196112


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +94,16 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    processMetrics.registerMetrics(registry);
+  }
+
+  public void initServerMetrics(MetricsProducer... producers) {

Review Comment:
   > @dlmarion What do you think is easier to understand? Classes that extended abstract server must call `super.registerMetrics` or call a method 'initServerMetrics' ? Either way requires something that may not be obvious.
   
   With Java, I think it's standard practice, or should be, for developers to  evaluate whether or not the superclass method needs to be called when overriding a method. 
   
   > 
   > Because of the tagging, the registration / initialization needs to occur in the run method - and after certain things have been initialized (host and port)
   > 
   agreed
   
   > I'm not thrilled with either approach, but went with a specific `initServerMetrics` method - if relying on calling `super.registerMetrics' and the needing to call the static `MetricsUtil.initializeProducers`which results in an indirect call to`registerMetrics`?
   > 
   > I was thinking about moving the static `initializeMetrics` out of MetricsUtil and put the functionality into AbstractServer - I think that functionality only applies to services extending AbstractServer.
   > 
   
   I think that might be fine iff only AbstractServer subclasses call that method.
   
   > The `initializeProducers` functionality needs to exist to register metrics producers for things that are created outside of the AbstarctServer context (thrift metrics, cache,....) - but those are created / exist within a service context that has already been expected to initialize the registry.
   
   agreed



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1163228539


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    lowMemoryMetricGuage =
+        Gauge
+            .builder(METRICS_APP_PREFIX + applicationName + "." + hostname + "."

Review Comment:
   There are tags - I'm trying to confirm that they are being set correctly as I look at Dave's other comments.



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] dlmarion commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1163957139


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +93,24 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    lowMemoryMetricGuage =
+        Gauge
+            .builder(METRICS_APP_PREFIX + applicationName + "." + hostname + "."

Review Comment:
   I agree, I think we want something like `accumulo.server.detected.low.memory` and the application name should be in a tag.



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1169988835


##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +94,16 @@ public void runServer() throws Exception {
     }
   }
 
+  @Override
+  public void registerMetrics(MeterRegistry registry) {
+    processMetrics.registerMetrics(registry);
+  }
+
+  public void initServerMetrics(MetricsProducer... producers) {

Review Comment:
   @dlmarion What do you think is easier to understand?  Classes that extended abstract server must call `super.registerMetrics` or call a method 'initServerMetrics' ?  Either way requires something that may not be obvious.  
   
   Because of the tagging, the registration / initialization needs to occur in the run method - and after certain things have been initialized (host and port) 
   
   I'm not thrilled with either approach, but went with a specific `initServerMetrics` method - if relying on calling `super.registerMetrics' and the needing to call the static `MetricsUtil.initializeProducers` which results in an indirect call to `registerMetrics`?
   
   I was thinking about moving the static `initializeMetrics` out of MetricsUtil and put the functionality into AbstractServer - I think that functionality only applies to services extending AbstractServer.  
   
   The `initializeProducers` functionality needs to exist to register metrics producers for things that are created outside of the AbstarctServer context (thrift metrics, cache,....) - but those are created / exist within a service context that has already been expected to initialize the registry.
   



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on a diff in pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1172946095


##########
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java:
##########
@@ -602,7 +602,7 @@ public void run() {
       LOG.error("Error initializing metrics, metrics will not be emitted.", e1);
     }
     pausedMetrics = new PausedCompactionMetrics();
-    MetricsUtil.initializeProducers(this, pausedMetrics);
+    initServerMetrics(pausedMetrics);

Review Comment:
   fixed in 49b93b349e



-- 
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-unsubscribe@accumulo.apache.org

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


[GitHub] [accumulo] EdColeman commented on pull request #3288: add low memory metric to abstract server

Posted by "EdColeman (via GitHub)" <gi...@apache.org>.
EdColeman commented on PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#issuecomment-1516763949

   @dlmarion can you see if 49b93b349e contains the changes you requested?  Or do you think there should be additional improvements?


-- 
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-unsubscribe@accumulo.apache.org

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