Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

draft Statements page + trace docs #6821

Merged
merged 6 commits into from
Apr 20, 2020
Merged

Conversation

taroface
Copy link
Contributor

@taroface taroface commented Mar 12, 2020

Fixes #4736.
Fixes #6941.

@taroface taroface requested a review from dhartunian March 12, 2020 23:41
@taroface taroface self-assigned this Mar 12, 2020
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@taroface taroface requested a review from piyush-singh March 19, 2020 20:17
Copy link
Contributor

@dhartunian dhartunian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@taroface just some minor suggestions. Thanks!


`INSERT INTO new_order(product_id, customer_id, no_w_id) VALUES (_, _, _)`
{{site.data.alerts.callout_info}}
For multiple SQL statements to be represented by a fingerprint, they must be identical aside from their values:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth disambiguating values here.

Statement | SQL statement [fingerprint](#sql-statement-fingerprints).<br><br>To view additional details of a SQL statement fingerprint, click this to open the [**Statement Details** page](#statement-details-page).
Txn Type | Type of transaction (implicit or explicit). Explicit transactions refer to statements that are wrapped by [`BEGIN`](begin-transaction.html) and [`COMMIT`](commit-transaction.html) statements by the client. Explicit transactions employ [transactional pipelining](architecture/transaction-layer.html#transaction-pipelining) and therefore report latencies that do not account for replication.<br><br>For statements not in explicit transactions, CockroachDB wraps each statement in individual implicit transactions.
Retries | Cumulative number of [retries](transactions.html#transaction-retries) of statements with this fingerprint within the last hour or specified [time interval](#time-interval).
Execution Count | Cumulative number of executions of statements with this fingerprint within the last hour or specified [time interval](#time-interval). <br><br>The bar indicates the ratio of runtime success (gray) to runtime failure (red) for the SQL statement fingerprint.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure there's ever a red line for failure here. I think this may be referring to the fact that the Retries are shown in red.


### Execution Count
When you activate diagnostics for a fingerprint, CockroachDB looks for SQL queries that match this fingerprint. On the next match, information about the SQL statement is written to a diagnostics bundle that you can download. This bundle consists of a JSON file that contains a distributed trace of the SQL statement.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth clarifying here that the diagnostics will be collected a maximum of N times for a given activated fingerprint where N is the number of nodes in your cluster?

Copy link

@piyush-singh piyush-singh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 6 of 6 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian and @taroface)


v20.1/admin-ui-statements-page.md, line 18 at r1 (raw file):

By default, the page shows SQL statements from all applications running on the cluster, and hides internal CockroachDB queries.

Use the **App** menu to filter the statements by [`application_name`](connection-parameters.html#additional-connection-parameters). If you haven't set `application_name` in the client connection string, it appears as `unset`. The menu also includes internal CockroachDB queries.

nit: maybe change "The menu also includes internal CockroachDB queries." to "CockroachDB's internal queries are only displayed under the (internal) app."

Perhaps worth calling out that queries from the sql shell also get their own app (not as important as the internal queries, but could be useful). SQL shell app is $ cockroach sql.


v20.1/admin-ui-statements-page.md, line 41 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

Might be worth disambiguating values here.

"literal values" maybe?


v20.1/admin-ui-statements-page.md, line 67 at r1 (raw file):

Execution Count | Cumulative number of executions of statements with this fingerprint within the last hour or specified [time interval](#time-interval). <br><br>The bar indicates the ratio of runtime success (gray) to runtime failure (red) for the SQL statement fingerprint.
Rows Affected | Average number of rows returned while executing statements with this fingerprint within the last hour or specified [time interval](#time-interval). <br><br>The gray bar indicates the mean number of rows returned. The blue bar indicates one standard deviation from the mean.
Latency | Average service latency of statements with this fingerprint within the last hour or specified [time interval](#time-interval). Service latency is the time taken to execute a query once it is received by the cluster. It does not include the time taken to return the result to the client. <br><br>The gray bar indicates the mean latency. The blue bar indicates one standard deviation from the mean.

nit: Doesn't include time to send the query to the cluster as well


v20.1/admin-ui-statements-page.md, line 68 at r1 (raw file):

Rows Affected | Average number of rows returned while executing statements with this fingerprint within the last hour or specified [time interval](#time-interval). <br><br>The gray bar indicates the mean number of rows returned. The blue bar indicates one standard deviation from the mean.
Latency | Average service latency of statements with this fingerprint within the last hour or specified [time interval](#time-interval). Service latency is the time taken to execute a query once it is received by the cluster. It does not include the time taken to return the result to the client. <br><br>The gray bar indicates the mean latency. The blue bar indicates one standard deviation from the mean.
Diagnostics | <span class="version-tag">New in v20.1:</span> Option to activate [diagnostics](#diagnostics) for this statement. If activated, this displays the status of diagnostics collection (`WAITING FOR QUERY`, `READY`, OR `ERROR`).

Worth noting that we will only expose the most recent trace here once this reads "Ready". If the user want to see a full history of diagnostics for a fingerprint they have to click through to statement details.


v20.1/admin-ui-statements-page.md, line 82 at r1 (raw file):

- **Total Time** is the cumulative time taken to execute statements with this fingerprint within the last hour or [specified time interval](#time-interval).
- **Mean Service Latency** is the average service latency of statements with this fingerprint within the last hour or [specified time interval](#time-interval).
- **App** displays the name specified by the [`application_name`](show-vars.html#supported-variables) session setting.

nit: Worth calling out that statements with the same fingerprint but different apps will not be aggregated? We might want to call this out above rather than in this section, but this made me realize we can see the same fingerprint multiple times if they are executed by different apps.


v20.1/admin-ui-statements-page.md, line 86 at r1 (raw file):

- **Distributed execution?** indicates whether the execution was distributed.
- **Used cost-based optimizer?** indicates whether the execution used the [cost-based optimizer](cost-based-optimizer.html).
- **Failed?** indicates whether the execution was successful.

nit: Similar to the above - we break fingerprints into two separate groups, successful attempts and unsuccessful attempts. This is because we expect the two categories to have different performance characteristics, and we don't want to lose the data about each of the cases individually by merging this data together. So if you run a query 2 times with one success and one failure, you would see 2 fingerprints pop up on the statements page. This is nuanced, so not sure we need this level of detail. Will defer to Ryan on whether to add anything about this.


v20.1/admin-ui-statements-page.md, line 104 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

is it worth clarifying here that the diagnostics will be collected a maximum of N times for a given activated fingerprint where N is the number of nodes in your cluster?

Oh this is actually a good point that I didn't think about. Probably worth mentioning.


v20.1/admin-ui-statements-page.md, line 104 at r1 (raw file):

CockroachDB looks for SQL queries

I'd be cautious with wording here. CRDB is actually waiting for the next query that matches the fingerprint to be run on any node in your cluster and checks each query you execute to see if it is a match. Searching, to me, implies the statement has been executed already and the cluster is looking for past executions.


v20.1/admin-ui-statements-page.md, line 122 at r1 (raw file):

{{site.data.alerts.end}}

Click **All statement diagnostics** to view a complete history of your collected diagnostics, each of which can be downloaded.

Worth calling out why we included this - although fingerprints are cleared hourly, we keep these diagnostics bundles forever. So if you need to access a bundle for a fingerprint that hasn't shown up in the past hour, you can navigate here to see a historical list of all bundles, regardless of whether the fingerprint has been purged from stats or not.


v20.1/admin-ui-statements-page.md, line 139 at r1 (raw file):

### Execution Stats

**Execution Latency by Phase** displays the service latency of statements matching this fingerprint, broken down by phase (parse, plan, run, overhead), as well as the overall service latency. The gray bar indicates the mean latency. The blue bar indicates one standard deviation from the mean.

We constantly get questions about what the "overhead" bucket includes in this breakdown. Right now it is a catchall - we take the overall latency, subtract parse, plan, and run latencies and whatever is left is assigned to overhead. This can include things like fetching table descriptors that were not cached, or other background tasks that might be required to get the query to execute. Since we are already updating docs for this page, might be worth including a note about that.


v20.1/admin-ui-statements-page.md, line 145 at r1 (raw file):

{{site.data.alerts.end}}

The **Statistics by Node** table provides a breakdown of the number of statements of the selected fingerprint per gateway node.

May be worth including a note about why this is useful. For example, this would catch the fact that you are executing queries on a node that is far from the data you are requesting. Can share an example if that helps.


v20.1/query-behavior-troubleshooting.md, line 25 at r1 (raw file):

use 

nit: use an

Copy link
Contributor Author

@taroface taroface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from

Quoted 43 lines of code…
  admin-ui-statements-page 
  to
  master       master     No branch found List is empty. 
          Mark as reviewed
        
          Show provisional diffs
              Combine commits for review    Combine commits for review
            of the overall amalgamated changes proposed for
        merging; not every commit may be accessible for diffing.
           Review each commit separately
            even if the changes were overwritten by a later
        commit; recommended only if commits are carefully organized.
            No elements found. Consider changing the search query. List is empty. 
            Previously reviewed revisions (r1)
            are immutable and won't be affected.
            For best results, set before starting review.
           Undo mark as reviewed
        
            Show full diff
          Admins can set a default for the repo.File Matrix
    δ 











  
    Review discussion
—

    Show
    4
    
    comments
  
      1 week ago
    tarofaceRyan Kuodraft Statements page + trace docs
> Fixes #4736. > Fixes #6941. > 1 week ago > tarofaceRyan Kuo

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian and @piyush-singh)


v20.1/admin-ui-statements-page.md, line 41 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

"literal values" maybe?

Done.


v20.1/admin-ui-statements-page.md, line 65 at r1 (raw file):

Previously, dhartunian (David Hartunian) wrote…

I'm not sure there's ever a red line for failure here. I think this may be referring to the fact that the Retries are shown in red.

Done.


v20.1/admin-ui-statements-page.md, line 68 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

Worth noting that we will only expose the most recent trace here once this reads "Ready". If the user want to see a full history of diagnostics for a fingerprint they have to click through to statement details.

Done.


v20.1/admin-ui-statements-page.md, line 82 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

nit: Worth calling out that statements with the same fingerprint but different apps will not be aggregated? We might want to call this out above rather than in this section, but this made me realize we can see the same fingerprint multiple times if they are executed by different apps.

Done. Added above.


v20.1/admin-ui-statements-page.md, line 86 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

nit: Similar to the above - we break fingerprints into two separate groups, successful attempts and unsuccessful attempts. This is because we expect the two categories to have different performance characteristics, and we don't want to lose the data about each of the cases individually by merging this data together. So if you run a query 2 times with one success and one failure, you would see 2 fingerprints pop up on the statements page. This is nuanced, so not sure we need this level of detail. Will defer to Ryan on whether to add anything about this.

Hmm... if a fingerprint executed unsuccessfully, how would this be indicated in the Statements list?


v20.1/admin-ui-statements-page.md, line 104 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

Oh this is actually a good point that I didn't think about. Probably worth mentioning.

Done.


v20.1/admin-ui-statements-page.md, line 104 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…
CockroachDB looks for SQL queries

I'd be cautious with wording here. CRDB is actually waiting for the next query that matches the fingerprint to be run on any node in your cluster and checks each query you execute to see if it is a match. Searching, to me, implies the statement has been executed already and the cluster is looking for past executions.

Done. Makes perfect sense!


v20.1/admin-ui-statements-page.md, line 122 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

Worth calling out why we included this - although fingerprints are cleared hourly, we keep these diagnostics bundles forever. So if you need to access a bundle for a fingerprint that hasn't shown up in the past hour, you can navigate here to see a historical list of all bundles, regardless of whether the fingerprint has been purged from stats or not.

Done.


v20.1/admin-ui-statements-page.md, line 139 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

We constantly get questions about what the "overhead" bucket includes in this breakdown. Right now it is a catchall - we take the overall latency, subtract parse, plan, and run latencies and whatever is left is assigned to overhead. This can include things like fetching table descriptors that were not cached, or other background tasks that might be required to get the query to execute. Since we are already updating docs for this page, might be worth including a note about that.

Done. Also added reference link to execution phases.


v20.1/admin-ui-statements-page.md, line 145 at r1 (raw file):

Previously, piyush-singh (Piyush Singh) wrote…

May be worth including a note about why this is useful. For example, this would catch the fact that you are executing queries on a node that is far from the data you are requesting. Can share an example if that helps.

Done and added link to https://www.cockroachlabs.com/docs/stable/make-queries-fast.html#schema-design - let me know if this isn't applicable here.

@taroface
Copy link
Contributor Author

Thanks for the awesome review @dhartunian @piyush-singh! Still had some follow-up questions on these.

Copy link
Contributor Author

@taroface taroface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian and @piyush-singh)


v20.1/admin-ui-statements-page.md, line 61 at r3 (raw file):

- <code style="white-space:pre-wrap">INSERT INTO new_order(product_id, customer_id, transaction_id) VALUES ($1, $2, $3)</code>

It is possible to see the same fingerprint listed multiple times in the following scenarios:

@piyush-singh Let me know what you think about this wording.

Copy link

@piyush-singh piyush-singh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian, @piyush-singh, and @taroface)


v20.1/admin-ui-statements-page.md, line 86 at r1 (raw file):

Previously, taroface (Ryan Kuo) wrote…

Hmm... if a fingerprint executed unsuccessfully, how would this be indicated in the Statements list?

We should see two fingerprints, one for executions that were failures and one for successes.


v20.1/admin-ui-statements-page.md, line 61 at r3 (raw file):

Previously, taroface (Ryan Kuo) wrote…

@piyush-singh Let me know what you think about this wording.

LGTM!

@taroface
Copy link
Contributor Author

@piyush-singh I have updated with the new bundle info whenever you want to do a (hopefully) final review!

Copy link

@piyush-singh piyush-singh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 2 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 2 of 2 files at r5.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian, @piyush-singh, and @taroface)

@taroface taroface requested a review from rmloveland April 16, 2020 18:30
Copy link
Contributor

@rmloveland rmloveland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with some annoying suggestions etc

@@ -7,135 +7,172 @@ toc: true
On a secure cluster, this area of the Admin UI can only be accessed by an `admin` user. See [Admin UI access](admin-ui-overview.html#admin-ui-access).
{{site.data.alerts.end}}

The **Statements** page helps you identify frequently executed or high latency [SQL statements](sql-statements.html). The **Statements** page also allows you to view the details of an individual SQL statement by clicking on the statement to view the **Statement Details** page.
The **Statements** page helps you identify frequently executed or high latency [SQL statements](sql-statements.html), view SQL statement [details](#statement-details-page), and download SQL statement [diagnostics](#diagnostics) for troubleshooting.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be easier to see these features/benefits as a bulleted list:

  • ID SQL statements
  • view statement details
  • download diagnostics


<img src="{{ 'images/v20.1/admin-ui-statements-page.png' | relative_url }}" alt="CockroachDB Admin UI Statements Page" style="border:1px solid #eee;max-width:100%" />
## Search and filter by application
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest either gerunding or not gerunding all the section titles, e.g., if this is "Search and filter" perhaps we should call the other section "Understand the statements page". (or "searching and filtering", "understanding").


The **Statements** page displays the details of the SQL statements executed within a specified time interval. At the end of the interval, the display is wiped clean, and you'll not see any statements on the page until the next set of statements is executed. By default, the time interval is set to one hour; however, you can customize the interval using the [`diagnostics.reporting.interval`](cluster-settings.html#settings) cluster setting.
Use the **App** menu to filter the statements by [`application_name`](connection-parameters.html#additional-connection-parameters). If you haven't set `application_name` in the client connection string, it appears as `unset`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest making this "to accomplish X, do Y" - aka "to filter the statements by application name, use the app filter"


## Limitation
By default, the page shows SQL statements from all applications running on the cluster, and hides internal CockroachDB queries.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest calling it "this page" as is done elsewhere

Use this page to identify SQL statements that you may want to [troubleshoot](query-behavior-troubleshooting.html). This might include statements that are experiencing high latencies, multiple [retries](transactions.html#transaction-retries), or execution failures. You can optionally create and retrieve [diagnostics](#diagnostics) for these statements.

{{site.data.alerts.callout_success}}
If you haven't yet executed any queries in the cluster as a user, this page will initially be blank.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important!!! Nice


The table below the statistics box provides the following details:
The **Statistics by Node** table provides a breakdown of the number of statements of the selected fingerprint per gateway node. You can use this table to determine whether, for example, you are executing queries on a node that is far from the data you are requesting (see [Make Queries Fast](make-queries-fast.html#schema-design)).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you are talking about whether a node is near or far from some data, I think the better section of 'Make Queries Fast' to link to is the one on cluster topology: make-queries-fast.html#cluster-topology

Used cost-based optimizer? | Indicates whether the statement (or multiple statements having the same fingerprint) were executed using the [cost-based optimizer](cost-based-optimizer.html).
Failed? | Indicate if the statement (or multiple statements having the same fingerprint) were executed successfully.
Node | ID of the gateway node.
Retries | Cumulative number of [retries](transactions.html#transaction-retries) of statements with this fingerprint within the last hour or specified [time interval](#time-interval).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha! Yes, this is the retries link I was mentioning above.

Latency | The average service latency of the SQL statement (or multiple statements having the same fingerprint) within the last hour or the [specified time interval](#limitation). <br><br>The latency is represented in two ways: The numerical value shows the mean latency, while the horizontal bar is color-coded (blue indicates the mean value and yellow indicates one standard deviation of the mean value of latency). The bar also helps you compare the mean latencies across all SQL fingerprints in the table. <br><br>You can sort the table by latency.
Statement | SQL statement [fingerprint](#sql-statement-fingerprints).<br><br>To view additional details of a SQL statement fingerprint, click this to open the [**Statement Details** page](#statement-details-page).
Txn Type | Type of transaction (implicit or explicit). Explicit transactions refer to statements that are wrapped by [`BEGIN`](begin-transaction.html) and [`COMMIT`](commit-transaction.html) statements by the client. Explicit transactions employ [transactional pipelining](architecture/transaction-layer.html#transaction-pipelining) and therefore report latencies that do not account for replication.<br><br>For statements not in explicit transactions, CockroachDB wraps each statement in individual implicit transactions.
Retries | Cumulative number of [retries](transactions.html#transaction-retries) of statements with this fingerprint within the last hour or specified [time interval](#time-interval).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These and other uses of "the last hour or specified time interval" could be changed to say only "the specified time interval [LINK]" so if/when the setting's default value changes, docs do not need to be updated. Judgment call

If you need help interpreting the output of the `EXPLAIN ANALYZE` statement, [contact us](file-an-issue.html).
You can also use an [`EXPLAIN ANALYZE`](explain-analyze.html) statement, which executes a SQL query and returns a physical query plan with execution statistics. Query plans provide information around SQL execution, which can be used to troubleshoot slow queries by figuring out where time is being spent, how long a processor (i.e., a component that takes streams of input rows and processes them according to a specification) is not doing work, etc.

If you need help interpreting the output of the `EXPLAIN ANALYZE` statement, [contact us](support-resources.html).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest changing "contact us" to "contact our support team" so it matches the phrasing in the previous paragraph; plus it makes clearer who / what exactly you are contacting


## Query is sometimes slow

If the query performance is irregular:

1. Run [`SHOW TRACE`](show-trace.html) for the query twice: once when the query is performing as expected and once when the query is slow.

2. [Contact us](file-an-issue.html) to analyze the outputs of the `SHOW TRACE` command.
2. [Contact us](support-resources.html) to analyze the outputs of the `SHOW TRACE` command.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same re: "contact us" -> "contact our support team"

Conversely, I guess everything could be changed to say "contact us". Your call ofc!

Copy link
Contributor Author

@taroface taroface left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @dhartunian, @piyush-singh, @rmloveland, and @taroface)


v20.1/admin-ui-statements-page.md, line 10 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

I think it would be easier to see these features/benefits as a bulleted list:

  • ID SQL statements
  • view statement details
  • download diagnostics

Done.


v20.1/admin-ui-statements-page.md, line 14 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

Suggest either gerunding or not gerunding all the section titles, e.g., if this is "Search and filter" perhaps we should call the other section "Understand the statements page". (or "searching and filtering", "understanding").

Nice catch! Thanks.


v20.1/admin-ui-statements-page.md, line 16 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

Suggest calling it "this page" as is done elsewhere

Done.


v20.1/admin-ui-statements-page.md, line 18 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

Suggest making this "to accomplish X, do Y" - aka "to filter the statements by application name, use the app filter"

Done.


v20.1/admin-ui-statements-page.md, line 44 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

This is long enough that I don't think it is really a note. It's more "regular content", IMO - specifically an overview of "how statement fingerprints work" (which btw is great).

Done.


v20.1/admin-ui-statements-page.md, line 71 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

I think these parameter descriptions need to be under the screenshot. They feel pretty disconnected way down here. Easy to forget what they are referring to, possibly.

Looking at the page, I think just straight-up moving them up so they are right under the screenshot they're referring to would work.

Done.


v20.1/admin-ui-statements-page.md, line 73 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

These and other uses of "the last hour or specified time interval" could be changed to say only "the specified time interval [LINK]" so if/when the setting's default value changes, docs do not need to be updated. Judgment call

Done.


v20.1/admin-ui-statements-page.md, line 81 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

I think it might be easier to jump down to what you want if these were a bulleted list. E.g.,

For each statement fingerprint, the details include:

  • overview
  • diagnostics
  • ... etc.

Done.


v20.1/admin-ui-statements-page.md, line 100 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

Question: Does this metric include both internal retries (performed by the DB) and retries that are seen by the user? Or just the latter?

In either case, the mention of retries should link to our transaction docs on retries, which does describe both types.

Done.


v20.1/admin-ui-statements-page.md, line 162 at r5 (raw file):

Previously, rmloveland (Rich Loveland) wrote…

Because you are talking about whether a node is near or far from some data, I think the better section of 'Make Queries Fast' to link to is the one on cluster topology: make-queries-fast.html#cluster-topology

Done.

@taroface taroface merged commit 2ed266e into master Apr 20, 2020
@taroface taroface deleted the admin-ui-statements-page branch April 20, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ui: Statements tracing Trace Information on Statements Page in Admin UI
5 participants