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 2020/11/10 21:13:32 UTC
[GitHub] [accumulo] milleruntime opened a new pull request #1772: Improve tool tip for hold time in Monitor
milleruntime opened a new pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772
* Also make the description for Ingest the same on all 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772#discussion_r520886460
##########
File path: server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/table.ftl
##########
@@ -44,7 +44,7 @@
<th title="Key/value pairs over each instance, table or tablet.">Entries </th>
<th title="The number of Key/Value pairs inserted. (Note that deletes are considered inserted)">Ingest </th>
<th title="The number of key/value pairs returned to clients. (Not the number of scans)">Query </th>
- <th title="The amount of time that ingest operations are suspended while waiting for data to be written to disk.">Hold Time </th>
+ <th title="The amount of time live ingest (mutations, batch writer) is suspended while waiting for tserver memory to free up. Writes are attempting to exceed tserver.memory.maps.max">Hold Time </th>
Review comment:
I like the first sentence, but I suggest: `s/is suspended/has been suspended/`.
I'm not sure I understand what the second sentence is trying to say. It may not be necessary. Or maybe it would be simpler to say something about the tserver awaiting a flush / minc?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772#discussion_r521077456
##########
File path: server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/table.ftl
##########
@@ -44,7 +44,7 @@
<th title="Key/value pairs over each instance, table or tablet.">Entries </th>
<th title="The number of Key/Value pairs inserted. (Note that deletes are considered inserted)">Ingest </th>
<th title="The number of key/value pairs returned to clients. (Not the number of scans)">Query </th>
- <th title="The amount of time that ingest operations are suspended while waiting for data to be written to disk.">Hold Time </th>
+ <th title="The amount of time live ingest (mutations, batch writer) is suspended while waiting for tserver memory to free up. Writes are attempting to exceed tserver.memory.maps.max">Hold Time </th>
Review comment:
Either of those would be fine... I didn't mean to omit the part that emphasized live ingest in my suggestion. I only meant to drop the part implying that a specific single property was the culprit, from the second sentence.
The only part I thought should be changed for the first sentence is to make it "past-tense and ongoing", rather than imply "present tense and instantaneous" ("has/have been suspended" vs. "is suspended"). Both of your last two suggestions encapsulate what I was going for there.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772#discussion_r520893774
##########
File path: server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/table.ftl
##########
@@ -44,7 +44,7 @@
<th title="Key/value pairs over each instance, table or tablet.">Entries </th>
<th title="The number of Key/Value pairs inserted. (Note that deletes are considered inserted)">Ingest </th>
<th title="The number of key/value pairs returned to clients. (Not the number of scans)">Query </th>
- <th title="The amount of time that ingest operations are suspended while waiting for data to be written to disk.">Hold Time </th>
+ <th title="The amount of time live ingest (mutations, batch writer) is suspended while waiting for tserver memory to free up. Writes are attempting to exceed tserver.memory.maps.max">Hold Time </th>
Review comment:
> I'm not sure I understand what the second sentence is trying to say. It may not be necessary. Or maybe it would be simpler to say something about the tserver awaiting a flush / minc?
I was trying to describe why it has a hold time in as few words possible. This seems to be the only spot in the code where we decide to hold commits up: https://github.com/apache/accumulo/blob/62110a3c27b9a27d8a7c0bc272aa6778910acad7/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java#L575
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772#discussion_r520911184
##########
File path: server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/table.ftl
##########
@@ -44,7 +44,7 @@
<th title="Key/value pairs over each instance, table or tablet.">Entries </th>
<th title="The number of Key/Value pairs inserted. (Note that deletes are considered inserted)">Ingest </th>
<th title="The number of key/value pairs returned to clients. (Not the number of scans)">Query </th>
- <th title="The amount of time that ingest operations are suspended while waiting for data to be written to disk.">Hold Time </th>
+ <th title="The amount of time live ingest (mutations, batch writer) is suspended while waiting for tserver memory to free up. Writes are attempting to exceed tserver.memory.maps.max">Hold Time </th>
Review comment:
Reading the docs for that property implies that it's not so simple as just this property. How, and by which properties, memory is managed, is complicated and can change over time, as that's implementation.
It might be better to stick to a high-level description, rather than pointing to specific properties / implementations, and explain that it's simply being held because its buffers are full and it needs to free up memory by flushing / minor compacting.
Also, these are just tool-tips, not supposed to be complete docs. I think "The duration over which commits have been held, waiting for the tserver to clear up memory."
I have reservations on your wording, because it didn't seem clear to me, and might be misleading, but I don't know what's best.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772#discussion_r520894176
##########
File path: server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/table.ftl
##########
@@ -44,7 +44,7 @@
<th title="Key/value pairs over each instance, table or tablet.">Entries </th>
<th title="The number of Key/Value pairs inserted. (Note that deletes are considered inserted)">Ingest </th>
<th title="The number of key/value pairs returned to clients. (Not the number of scans)">Query </th>
- <th title="The amount of time that ingest operations are suspended while waiting for data to be written to disk.">Hold Time </th>
+ <th title="The amount of time live ingest (mutations, batch writer) is suspended while waiting for tserver memory to free up. Writes are attempting to exceed tserver.memory.maps.max">Hold Time </th>
Review comment:
And provide a pointer to the configuration property determining when to hold.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772#discussion_r520925883
##########
File path: server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/table.ftl
##########
@@ -44,7 +44,7 @@
<th title="Key/value pairs over each instance, table or tablet.">Entries </th>
<th title="The number of Key/Value pairs inserted. (Note that deletes are considered inserted)">Ingest </th>
<th title="The number of key/value pairs returned to clients. (Not the number of scans)">Query </th>
- <th title="The amount of time that ingest operations are suspended while waiting for data to be written to disk.">Hold Time </th>
+ <th title="The amount of time live ingest (mutations, batch writer) is suspended while waiting for tserver memory to free up. Writes are attempting to exceed tserver.memory.maps.max">Hold Time </th>
Review comment:
Or "The amount of time live ingest operations (mutations, batch writes) have been waiting for the tserver to free up memory"
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #1772: Improve tool tip for hold time in Monitor
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #1772:
URL: https://github.com/apache/accumulo/pull/1772#discussion_r520914867
##########
File path: server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/table.ftl
##########
@@ -44,7 +44,7 @@
<th title="Key/value pairs over each instance, table or tablet.">Entries </th>
<th title="The number of Key/Value pairs inserted. (Note that deletes are considered inserted)">Ingest </th>
<th title="The number of key/value pairs returned to clients. (Not the number of scans)">Query </th>
- <th title="The amount of time that ingest operations are suspended while waiting for data to be written to disk.">Hold Time </th>
+ <th title="The amount of time live ingest (mutations, batch writer) is suspended while waiting for tserver memory to free up. Writes are attempting to exceed tserver.memory.maps.max">Hold Time </th>
Review comment:
I wanted to make more clear to admin what is happening for this number to be high. To make it clear that live ingest is held up vs bulk imports.
What about "The duration live ingest ops (mutations, batch writes) have been held while waiting for tserver to free up memory"
----------------------------------------------------------------
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:
users@infra.apache.org