You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/09/22 12:54:03 UTC
[GitHub] [accumulo] milleruntime opened a new pull request #2283: Add Active Compactions page to Monitor
milleruntime opened a new pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283
* Closes #2230
* Add code required to populate active compactions on new page
* Rename scans page title to Active Scans
* Rename ScansResource method to getActiveScans()
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-925748285
Created #2286 for fixing text alignment across all tables.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927837227
> What is "Oldest Age" column in the UI? Is that the time since the last response time from the tserver? Are you planning on showing anything other than a count?
That is the age returned by the thrift object `ActiveCompaction`.
> `ThriftClientHandler.getActiveCompactions` only returns information about the compactions running internally. However, the TabletServer also has information on the compactions that are running externally. Conveying just the count of internal and external metrics might require a new Thrift endpoint though as I don't think the information will fit into `getActiveCompactions`.
This would be a good opportunity to include External compaction information as well. Now that you mention it, I don't think the Monitor has anything about External compactions.
--
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 #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927829457
What is "Oldest Age" column in the UI? Is that the time since the last response time from the tserver? Are you planning on showing anything other than a count? `ThriftClientHandler.getActiveCompactions` only returns information about the compactions running internally. However, the TabletServer also has information on the compactions that are running externally. Conveying just the count of internal and external metrics might require a new Thrift endpoint though as I don't think the information will fit into `getActiveCompactions`.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927948405
Another option would be to add a "Fetch Compactions" button on the Monitor to make it a manual process performed by the admin.
--
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 #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927971612
`ManagerMonitorInfo.tServerInfo.tableMap` values are `TableInfo` objects which contain the number of queued and running major compactions. Just need to sum them up for each tserver.
--
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] milleruntime commented on a change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r716625052
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -553,7 +565,24 @@ private ServletHolder getRestServlet() {
}
}
+ public static class CompactionStats {
+ public final long count;
+ public final Long oldest;
+ public final long fetched;
+
+ CompactionStats(List<ActiveCompaction> active) {
+ this.count = active.size();
+ long oldest = -1;
+ for (ActiveCompaction a : active) {
+ oldest = Math.max(oldest, a.age);
+ }
+ this.oldest = oldest < 0 ? null : oldest;
+ this.fetched = System.currentTimeMillis();
Review comment:
A lot of this code was copy and pasted but I can do a change replacing `System.currentTimeMillis()` in the Monitor.
--
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 change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r716989294
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -484,6 +485,17 @@ public void run() {
}
}).start();
+ Threads.createThread("Compaction fetcher", () -> {
+ while (true) {
+ try {
+ fetchCompactions();
+ } catch (Exception e) {
+ log.warn("{}", e.getMessage(), e);
+ }
+ sleepUninterruptibly(5, TimeUnit.SECONDS);
+ }
+ }).start();
+
Review comment:
> This is also currently being done for active scans. So if it's an issue with the fetch compactions thread, we should do something with the scans fetch as well.
Yeah, I saw that. It seems like it's a problem there, too. I'm not sure when that got added, but we definitely don't need the monitor constantly talking to every server in the cluster every 5 seconds, if the monitor doesn't even have any active clients requesting pages.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927954455
> The LiveTServerSet in the Manager is already collecting the stats, that won't work?
Possibly. I know that I have had issues getting that information from the Manager in the past.
--
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] Manno15 commented on a change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
Manno15 commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r714307312
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionInfo.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.monitor.rest.compactions;
+
+import org.apache.accumulo.core.master.thrift.TabletServerStatus;
+
+/**
+ * Generates a compaction info JSON object
+ *
+ * @since 2.0.0
Review comment:
Should this be `@since 2.1.0`?
--
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 edited a comment on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion edited a comment on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927995637
Can you use the last contact field in the TabletServerStatus? I assume that is what the `/tservers` display is using.
EDIT: Nevermind, I forgot that you also want the age of the compaction, not just the number on the tserver. For external compactions I'm not sure the age will always be correct. For example, If the tserver restarts and the external compaction continues.
--
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 #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927952695
The LiveTServerSet in the Manager is already collecting the stats, that won't work?
--
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 #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927995637
Can you use the last contact field in the TabletServerStatus? I assume that is what the `/tables` display is using.
--
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] milleruntime commented on a change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r716650518
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -484,6 +485,17 @@ public void run() {
}
}).start();
+ Threads.createThread("Compaction fetcher", () -> {
+ while (true) {
+ try {
+ fetchCompactions();
+ } catch (Exception e) {
+ log.warn("{}", e.getMessage(), e);
+ }
+ sleepUninterruptibly(5, TimeUnit.SECONDS);
+ }
+ }).start();
+
Review comment:
> The problem with this pattern is that there's a constant background thread doing RPC calls to the tservers to collect these stats, even if nobody is accessing the page on the monitor. And, it has to talk to every tserver, of which there could be thousands. This is a lot of constant activity that might not even be needed. The general pattern of the monitor is to avoid refreshing data if nobody is accessing the page, and to limit the refresh of the data shown on the page to collect it no more frequently than once every 5 seconds. This is how we collect the "mmi" object, anyway.
>
> Fast display should not bog down the cluster like this.
This is also currently being done for active scans. So if it's an issue with the fetch compactions thread, we should do something with the scans fetch as well.
--
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 change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r714964100
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -553,7 +565,24 @@ private ServletHolder getRestServlet() {
}
}
+ public static class CompactionStats {
+ public final long count;
+ public final Long oldest;
+ public final long fetched;
+
+ CompactionStats(List<ActiveCompaction> active) {
+ this.count = active.size();
+ long oldest = -1;
+ for (ActiveCompaction a : active) {
+ oldest = Math.max(oldest, a.age);
+ }
+ this.oldest = oldest < 0 ? null : oldest;
+ this.fetched = System.currentTimeMillis();
Review comment:
This should use nano time to compute time durations.
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -484,6 +485,17 @@ public void run() {
}
}).start();
+ Threads.createThread("Compaction fetcher", () -> {
+ while (true) {
+ try {
+ fetchCompactions();
+ } catch (Exception e) {
+ log.warn("{}", e.getMessage(), e);
+ }
+ sleepUninterruptibly(5, TimeUnit.SECONDS);
+ }
+ }).start();
+
Review comment:
The problem with this pattern is that there's a constant background thread doing RPC calls to the tservers to collect these stats, even if nobody is accessing the page on the monitor. And, it has to talk to every tserver, of which there could be thousands. This is a lot of constant activity that might not even be needed. The general pattern of the monitor is to avoid refreshing data if nobody is accessing the page, and to limit the refresh of the data shown on the page to collect it no more frequently than once every 5 seconds. This is how we collect the "mmi" object, anyway.
Fast display should not bog down the cluster like this.
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -589,6 +624,33 @@ private void fetchScans() throws Exception {
}
}
+ private void fetchCompactions() throws Exception {
+ ServerContext context = getContext();
+ for (String server : context.instanceOperations().getTabletServers()) {
+ final HostAndPort parsedServer = HostAndPort.fromString(server);
+ Client tserver = ThriftUtil.getTServerClient(parsedServer, context);
+ try {
+ var compacts = tserver.getActiveCompactions(null, context.rpcCreds());
+ synchronized (allCompactions) {
+ allCompactions.put(parsedServer, new CompactionStats(compacts));
+ }
+ } catch (Exception ex) {
+ log.debug("Failed to get active compactions from {}", server, ex);
+ } finally {
+ ThriftUtil.returnClient(tserver);
+ }
+ }
+ // Age off old compaction information
+ var entryIter = allCompactions.entrySet().iterator();
+ long now = System.currentTimeMillis();
+ while (entryIter.hasNext()) {
+ var entry = entryIter.next();
+ if (now - entry.getValue().fetched > 5 * 60 * 1000) {
Review comment:
This should use nanoTime to compute time durations, to avoid system clock adjustments affecting the computation.
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionInfo.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.monitor.rest.compactions;
+
+import org.apache.accumulo.core.master.thrift.TabletServerStatus;
+
+/**
+ * Generates a compaction info JSON object
+ *
+ * @since 2.1.0
+ */
+public class CompactionInfo {
+
+ // Variable names become JSON keys
+ public String server;
+
+ public long count;
+ public Long oldest;
+
+ public CompactionInfo() {}
+
+ /**
+ * Stores new compaction information
+ *
+ * @param tserverInfo
+ * status of the tserver
+ * @param count
+ * number of compactions
+ * @param oldest
+ * time of oldest compaction
+ */
+ public CompactionInfo(TabletServerStatus tserverInfo, long count, Long oldest) {
Review comment:
Not sure, but OptionalLong might be better here, rather than passing a nullable boxed version of Long.
--
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] milleruntime merged pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927804006
> @milleruntime I have performance concerns about constantly collecting information from RPC calls to every tablet server every 5 seconds. I think that could be handled better.
I could make the stats for the page disabled by default.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-925743518
> The monitor does seem slightly inconsistent with text alignment (inside each column)
Good catch. The default text alignment for datatables is different from our custom jquery generated tables. So it might be easier to fix them across all pages. I can do that in a separate commit.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-930484037
I created #2294 as a follow on for the concerns discussed in this PR.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927994278
Yeah that is what we do for the tables page in https://github.com/apache/accumulo/blob/f2b0936029fcaadb925bd9c229403c82f0283211/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/tables/TablesResource.java#L91
But that doesn't have the age of the compactions. That is in a separate RPC call for some reason.
--
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 change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r716989294
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -484,6 +485,17 @@ public void run() {
}
}).start();
+ Threads.createThread("Compaction fetcher", () -> {
+ while (true) {
+ try {
+ fetchCompactions();
+ } catch (Exception e) {
+ log.warn("{}", e.getMessage(), e);
+ }
+ sleepUninterruptibly(5, TimeUnit.SECONDS);
+ }
+ }).start();
+
Review comment:
> This is also currently being done for active scans. So if it's an issue with the fetch compactions thread, we should do something with the scans fetch as well.
Yeah, I saw that. It seems like it's a problem there, too. I'm not sure when that got added, but we definitely don't need the monitor constantly talking to every server in the cluster every 5 seconds, if the monitor doesn't even have any active clients requesting pages.
--
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 #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927817128
The utility of seeing running compactions seems that it would be highest for long running compactions - polling on an order of minutes might provide enough fidelity to still be usefull.
--
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] milleruntime commented on a change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r716998405
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -484,6 +485,17 @@ public void run() {
}
}).start();
+ Threads.createThread("Compaction fetcher", () -> {
+ while (true) {
+ try {
+ fetchCompactions();
+ } catch (Exception e) {
+ log.warn("{}", e.getMessage(), e);
+ }
+ sleepUninterruptibly(5, TimeUnit.SECONDS);
+ }
+ }).start();
+
Review comment:
I am working on the idea to have a button to fetch the info. I like the manual fetch in this case. We can still restrict it to only fetch every minute so an Admin can't spam the tservers. This way if no one cares, there won't ever be any wasted work. If a user does care, they will have the info provided on the page through an AJAX call.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-925110337
> @milleruntime can you share a screenshot of what this looks like?
It looks almost identical to the Active Scans page but I used datatables for the table
.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-928217554
FYI I created a follow on to include something about external compactions in the monitor: https://github.com/apache/accumulo/issues/2290
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927964447
> The LiveTServerSet in the Manager is already collecting the stats, that won't work?
The Monitor uses the `ManagerMonitorInfo` object to get all its information by calling getManagerStats() on the manager client.
--
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 edited a comment on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion edited a comment on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927995637
Can you use the last contact field in the TabletServerStatus? I assume that is what the `/tservers` display is using.
--
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 #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
EdColeman commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927952653
One thing that is nice when a manual (or slow refresh) is to also display the time that the page was fetched - that way you can determine when the results were actually gathered.
--
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] keith-turner commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
keith-turner commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-925010709
@milleruntime can you share a screenshot of what this looks like?
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-930484037
I created #2294 as a follow on for the concerns discussed in this PR.
--
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] milleruntime commented on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-928217554
FYI I created a follow on to include something about external compactions in the monitor: https://github.com/apache/accumulo/issues/2290
--
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] milleruntime commented on a change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r716998405
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
##########
@@ -484,6 +485,17 @@ public void run() {
}
}).start();
+ Threads.createThread("Compaction fetcher", () -> {
+ while (true) {
+ try {
+ fetchCompactions();
+ } catch (Exception e) {
+ log.warn("{}", e.getMessage(), e);
+ }
+ sleepUninterruptibly(5, TimeUnit.SECONDS);
+ }
+ }).start();
+
Review comment:
I am working on the idea to have a button to fetch the info. I like the manual fetch in this case. We can still restrict it to only fetch every minute so an Admin can't spam the tservers. This way if no one cares, there won't ever be any wasted work. If a user does care, they will have the info provided on the page through an AJAX call.
--
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 edited a comment on pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion edited a comment on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927838131
IIRC, the compactions queued and running in the `tables` view does include external compactions. I'm tracing through the code now, it looks like the `tables` view gets that information from the Manager.
EDIT: It looks like the `LiveTServerSet` in the `Manager` calls `ThriftClientHandler.getTabletServerStatus()` , which calls `TabletServer.getStats()`. I wonder if you could just make the same call to the Manager...
--
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 #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
dlmarion commented on pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#issuecomment-927838131
IIRC, the compactions queued and running in the `tables` view does include external compactions. I'm tracing through the code now, it looks like the `tables` view gets that information from the Manager.
--
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] Manno15 commented on a change in pull request #2283: Add Active Compactions page to Monitor
Posted by GitBox <gi...@apache.org>.
Manno15 commented on a change in pull request #2283:
URL: https://github.com/apache/accumulo/pull/2283#discussion_r714307732
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/Compactions.java
##########
@@ -0,0 +1,43 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.monitor.rest.compactions;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Generates a new list of compactions as a JSON object
+ *
+ * @since 2.1
Review comment:
```suggestion
* @since 2.1.0
```
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionInfo.java
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.monitor.rest.compactions;
+
+import org.apache.accumulo.core.master.thrift.TabletServerStatus;
+
+/**
+ * Generates a compaction info JSON object
+ *
+ * @since 2.0.0
Review comment:
Should this be `@since 2.1.0`? Same with the
##########
File path: server/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionsResource.java
##########
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.accumulo.monitor.rest.compactions;
+
+import java.util.Map;
+
+import jakarta.inject.Inject;
+import jakarta.ws.rs.GET;
+import jakarta.ws.rs.Path;
+import jakarta.ws.rs.Produces;
+import jakarta.ws.rs.core.MediaType;
+
+import org.apache.accumulo.core.manager.thrift.ManagerMonitorInfo;
+import org.apache.accumulo.core.master.thrift.TabletServerStatus;
+import org.apache.accumulo.core.util.HostAndPort;
+import org.apache.accumulo.monitor.Monitor;
+
+/**
+ * Generate a new Compaction list JSON object
+ *
+ * @since 2.0.0
Review comment:
Same as the above comment.
--
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