Add `bytes` into "Query parameters" for CAT Segment Replication API #4731

tlfeng · 2023-08-09T22:22:31Z

Description

Add description for bytes parameter into "Query parameters" section for CAT Segment Replication API
This is a common parameters for CAT API, and I think it's worth to mention here, because it's available for "cat-segment-replication" REST API.
Correct the description for time parameter. The default value is not milliseconds, but no default value. By default, the response is human-formatted, for example, 1.2s instead of 1209. Besides, the "data type" is not Time "value", but Time "unit".

An example for the API response:

$ curl "opens-clust-xxx.elb.us-west-2.amazonaws.com/_cat/segment_replication?v&pretty"
shardId          target_node                              target_host checkpoints_behind bytes_behind current_lag last_completed_lag rejected_requests
[logs-221998][7] ip-10-0-4-86.us-west-2.compute.internal  10.0.4.86   1                  47.3kb       988ms       823ms              0
[logs-221998][7] ip-10-0-3-170.us-west-2.compute.internal 10.0.3.170  1                  39.2kb       988ms       1.2s               0
[logs-231998][0] ip-10-0-3-66.us-west-2.compute.internal  10.0.3.66   1                  31.6kb       791ms       699ms              0
[logs-231998][0] ip-10-0-4-187.us-west-2.compute.internal 10.0.4.187  1                  40kb         791ms       1.1s               0
[logs-231998][0] ip-10-0-3-170.us-west-2.compute.internal 10.0.3.170  1                  40kb         791ms       342ms              0
[logs-231998][0] ip-10-0-4-86.us-west-2.compute.internal  10.0.4.86   1                  31.6kb       791ms       343ms              0

In addition, the format and description in the CAT Segment Replication API is a bit distinct with other CAT APIs, such as CAT Recovery.
1 There is no "code" (``) style for the parameter names for the other pages.
2 The descriptions for the parameters are different from other pages.
I didn't make changes to make "cat-segment-replication" align with other pages.

Issues Resolved

n/a

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Tianli Feng <ftianli@amazon.com>

kolchfa-aws

Thanks, @tlfeng! Could you clarify the default values for these? An example could help as well.

kolchfa-aws · 2023-08-10T00:52:47Z

_api-reference/cat/cat-segment-replication.md

@@ -39,10 +39,11 @@ Parameter | Data type | Description
 `active_only` | Boolean | If `true`, the response only includes active segment replications. Defaults to `false`. 
 [`detailed`](#additional-detailed-response-metrics) | String | If `true`, the response includes additional metrics for each stage of a segment replication event. Defaults to `false`.
 `shards` | String | A comma-separated list of shards to display.
+`byte` | Byte size | [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display byte size values.


Is there a default for this parameter? Since it's optional, I assume there should be a default units to display byte values.

Suggested change

`byte` | Byte size | [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display byte size values.

`byte` | Byte units | [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display byte size values.

Sorry to show the Elastic's document https://www.elastic.co/guide/en/elasticsearch/reference/7.10/cat.html#numeric-formats, but that can make the explanation easier.

By default, the value of "bytes" and "time" fields are formatted to be human-readable. The value will be added with the most suitable unit.
For example:
When file size larger than 1000 byte, the unit will be shown as "kb", otherwise shown as "b".
When the time is more than 1000 milliseconds, the unit will be shown as "s", otherwise shown as "ms".
When the number is even larger, a higher-level unit will be shown.

$ curl "opens-clust-xxx.elb.us-west-2.amazonaws.com/_cat/segment_replication?v" shardId target_node target_host checkpoints_behind bytes_behind current_lag last_completed_lag rejected_requests [logs-221998][7] ip-10-0-4-86.us-west-2.compute.internal 10.0.4.86 1 3b 988ms 823ms 0 [logs-221998][7] ip-10-0-3-170.us-west-2.compute.internal 10.0.3.170 1 39.2kb 988ms 1.2s 0 [logs-231998][0] ip-10-0-3-66.us-west-2.compute.internal 10.0.3.66 1 31.6kb 791ms 699ms 0

This is an example of the API response when assigning the parameters time=ms and bytes=b, the value will be purely numeric and directly comparable.

$ curl "opens-clust-xxx.elb.us-west-2.amazonaws.com/_cat/segment_replication?v&time=ms&bytes=b" shardId target_node target_host checkpoints_behind bytes_behind current_lag last_completed_lag rejected_requests [logs-221998][0] ip-10-0-5-102.us-west-2.compute.internal 10.0.5.102 1 7480 1116 1664 0 [logs-221998][0] ip-10-0-3-36.us-west-2.compute.internal 10.0.3.36 1 16034 1116 1316 0 [logs-221998][0] ip-10-0-4-86.us-west-2.compute.internal 10.0.4.86 1 16034 1116 2018 0

The source codes of adding the most suitable unit when output are here:
https://github.com/opensearch-project/OpenSearch/blob/7278f434cd83f095623e34c2105f1b79d247af18/libs/core/src/main/java/org/opensearch/core/common/unit/ByteSizeValue.java#L166
https://github.com/opensearch-project/OpenSearch/blob/7278f434cd83f095623e34c2105f1b79d247af18/libs/common/src/main/java/org/opensearch/common/unit/TimeValue.java#L251

kolchfa-aws · 2023-08-10T00:54:01Z

_api-reference/cat/cat-segment-replication.md

 `format` | String | A short version of the HTTP accept header. Valid values include `JSON` and `YAML`.  
 `h` | String | A comma-separated list of column names to display. 
 `help` | Boolean | If `true`, the response includes help information. Defaults to `false`.
-`time` | Time value | [Units]({{site.url}}{{site.baseurl}}/opensearch/units) used to display time values. Defaults to `ms` (milliseconds).
+`time` | Time | [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display time values.


Same in this line... if time is not specified, what are the units used to display time values?

Suggested change

`time` | Time | [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display time values.

`time` | Time units | [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display time values.

Please see my above comment #4731 (comment). Forgot to add a clear example for the default situation earlier.

Signed-off-by: Tianli Feng <ftianli@amazon.com>

kolchfa-aws

Thanks a lot for the explanation. In this case, I think "Time units" and "Byte units" makes the most sense for the data types of these parameters. I'd also like to add an explanation that you provided and the examples to the documentation but I can address that in another PR.

tlfeng · 2023-08-10T05:53:06Z

Thanks a lot for the explanation. In this case, I think "Time units" and "Byte units" makes the most sense for the data types of these parameters. I'd also like to add an explanation that you provided and the examples to the documentation but I can address that in another PR.

It make sense! I also thought about using "Time/Byte Unit" as the data type. I finally changed my mind to make inconsistent with other CAT API pages.
I update the data type in the new commit 89f57af.
Since "Byte units" becomes the longest text in the column, the length of this column has been enlarged by one space, which make every line in the table got changed. 😅

…d 'Time unites' Signed-off-by: Tianli Feng <ftianli@amazon.com>

kolchfa-aws

LGTM. Thanks, @tlfeng!

…4731) * Add `byte` into "Query parameters" for CAT Segment Replication API Signed-off-by: Tianli Feng <ftianli@amazon.com> * Add a trailing slash to the links Signed-off-by: Tianli Feng <ftianli@amazon.com> * Correct the parameter name to bytes Signed-off-by: Tianli Feng <ftianli@amazon.com> * Change data type of 'bytes' and 'time' parameters to 'Byte unites' and 'Time unites' Signed-off-by: Tianli Feng <ftianli@amazon.com> --------- Signed-off-by: Tianli Feng <ftianli@amazon.com> (cherry picked from commit 4d6106a) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…4731) (#4735) * Add `byte` into "Query parameters" for CAT Segment Replication API * Add a trailing slash to the links * Correct the parameter name to bytes * Change data type of 'bytes' and 'time' parameters to 'Byte unites' and 'Time unites' --------- (cherry picked from commit 4d6106a) Signed-off-by: Tianli Feng <ftianli@amazon.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

…pensearch-project#4731) * Add `byte` into "Query parameters" for CAT Segment Replication API Signed-off-by: Tianli Feng <ftianli@amazon.com> * Add a trailing slash to the links Signed-off-by: Tianli Feng <ftianli@amazon.com> * Correct the parameter name to bytes Signed-off-by: Tianli Feng <ftianli@amazon.com> * Change data type of 'bytes' and 'time' parameters to 'Byte unites' and 'Time unites' Signed-off-by: Tianli Feng <ftianli@amazon.com> --------- Signed-off-by: Tianli Feng <ftianli@amazon.com>

…4731) * Add `byte` into "Query parameters" for CAT Segment Replication API Signed-off-by: Tianli Feng <ftianli@amazon.com> * Add a trailing slash to the links Signed-off-by: Tianli Feng <ftianli@amazon.com> * Correct the parameter name to bytes Signed-off-by: Tianli Feng <ftianli@amazon.com> * Change data type of 'bytes' and 'time' parameters to 'Byte unites' and 'Time unites' Signed-off-by: Tianli Feng <ftianli@amazon.com> --------- Signed-off-by: Tianli Feng <ftianli@amazon.com>

Add byte into "Query parameters" for CAT Segment Replication API

815116c

Signed-off-by: Tianli Feng <ftianli@amazon.com>

tlfeng requested review from cwillum, hdhalter, kolchfa-aws, Naarcha-AWS, vagimeli, ananzh, seanneumann, AMoo-Miki and natebower as code owners August 9, 2023 22:22

Add a trailing slash to the links

a8486c0

Signed-off-by: Tianli Feng <ftianli@amazon.com>

kolchfa-aws reviewed Aug 10, 2023

View reviewed changes

Correct the parameter name to bytes

e562be3

Signed-off-by: Tianli Feng <ftianli@amazon.com>

tlfeng changed the title ~~Add byte into "Query parameters" for CAT Segment Replication API~~ Add bytes into "Query parameters" for CAT Segment Replication API Aug 10, 2023

kolchfa-aws self-assigned this Aug 10, 2023

kolchfa-aws approved these changes Aug 10, 2023

View reviewed changes

Change data type of 'bytes' and 'time' parameters to 'Byte unites' an…

89f57af

…d 'Time unites' Signed-off-by: Tianli Feng <ftianli@amazon.com>

tlfeng mentioned this pull request Aug 10, 2023

Capture and report replication lag metric opensearch-project/opensearch-benchmark#339

Closed

kolchfa-aws approved these changes Aug 10, 2023

View reviewed changes

kolchfa-aws added the backport 2.9 PR: Backport label for 2.9 label Aug 10, 2023

kolchfa-aws merged commit 4d6106a into opensearch-project:main Aug 10, 2023

opensearch-trigger-bot bot mentioned this pull request Aug 10, 2023

[Backport 2.9] Add bytes into "Query parameters" for CAT Segment Replication API #4735

Merged

tlfeng deleted the byte-cat-segrep-api branch August 10, 2023 16:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `bytes` into "Query parameters" for CAT Segment Replication API #4731

Add `bytes` into "Query parameters" for CAT Segment Replication API #4731

tlfeng commented Aug 9, 2023 •

edited

Loading

kolchfa-aws left a comment

kolchfa-aws Aug 10, 2023

kolchfa-aws Aug 10, 2023

tlfeng Aug 10, 2023 •

edited

Loading

kolchfa-aws Aug 10, 2023

tlfeng Aug 10, 2023

kolchfa-aws left a comment

tlfeng commented Aug 10, 2023 •

edited

Loading

kolchfa-aws left a comment

	`byte` \| Byte size \| [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display byte size values.
	`byte` \| Byte units \| [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display byte size values.

	`time` \| Time \| [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display time values.
	`time` \| Time units \| [Units]({{site.url}}{{site.baseurl}}/opensearch/units/) used to display time values.

Add bytes into "Query parameters" for CAT Segment Replication API #4731

Add bytes into "Query parameters" for CAT Segment Replication API #4731

Conversation

tlfeng commented Aug 9, 2023 • edited Loading

Description

Issues Resolved

Checklist

kolchfa-aws left a comment

Choose a reason for hiding this comment

kolchfa-aws Aug 10, 2023

Choose a reason for hiding this comment

kolchfa-aws Aug 10, 2023

Choose a reason for hiding this comment

tlfeng Aug 10, 2023 • edited Loading

Choose a reason for hiding this comment

kolchfa-aws Aug 10, 2023

Choose a reason for hiding this comment

tlfeng Aug 10, 2023

Choose a reason for hiding this comment

kolchfa-aws left a comment

Choose a reason for hiding this comment

tlfeng commented Aug 10, 2023 • edited Loading

kolchfa-aws left a comment

Choose a reason for hiding this comment

Add `bytes` into "Query parameters" for CAT Segment Replication API #4731

Add `bytes` into "Query parameters" for CAT Segment Replication API #4731

tlfeng commented Aug 9, 2023 •

edited

Loading

tlfeng Aug 10, 2023 •

edited

Loading

tlfeng commented Aug 10, 2023 •

edited

Loading