-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Segment Replication] Implementing cat/segment_replication API #5718
[Segment Replication] Implementing cat/segment_replication API #5718
Conversation
Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Rishikesh1159 <[email protected]>
…h1159/OpenSearch into segment_replication_API
Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Rishikesh1159 <[email protected]>
Signed-off-by: Rishikesh1159 <[email protected]>
…h1159/OpenSearch into segment_replication_API
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Rishikesh1159 <[email protected]>
Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
* compatible open source license. | ||
*/ | ||
|
||
package org.opensearch.action.admin.indices.segment_replication; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't catch this on first run through, lets pls remove this snake casing in the package name. How about:
org.opensearch.action.admin.indices.replication
/** | ||
*Indices segment replication | ||
*/ | ||
ActionFuture<SegmentReplicationStatsResponse> segment_replication(SegmentReplicationStatsRequest request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets pls use camel casing here and below segmentReplication
.
@Override | ||
public String getSourceDescription() { | ||
String description = "Host:" + this.sourceNode.getHostName() + ", Node:" + this.sourceNode.getName(); | ||
return description; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - no need for one time use variable. return "Host:" + this.sourceNode.getHostName() + ", Node:" + this.sourceNode.getName();
Also I think we only need the node name, not the host name because that can be looked up from other APIs.
/** | ||
* Get the source description | ||
*/ | ||
default String getSourceDescription() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given this is going to be displayed directly in a rest API, I do not think we should provide a default here and require an implementation.
return timingData; | ||
} | ||
|
||
public Stage getStage() { | ||
return stage; | ||
public long getGetCheckpointInfoStageTime() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These methods can return a TimeValue to reduce some duplication.
builder.startObject(SegmentReplicationState.Fields.INDEX); | ||
index.toXContent(builder, params); | ||
builder.endObject(); | ||
builder.field(Fields.REPLICATING_STAGE, new TimeValue(timingData.get("REPLICATING"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than fetching from the map here directly, you can invoke your methods above. getGetCheckpointInfoStageTime
etc...
* returns SegmentReplicationState of on-going segment replication events. | ||
*/ | ||
public SegmentReplicationState getOngoingEventSegmentReplicationState(ShardRouting shardRouting) { | ||
SegmentReplicationTarget target = onGoingReplications.getOngoingReplicationTarget(shardRouting.shardId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add @Nullable
annotation to these method declarations. Also all three of these methods only need the ShardId, not the entire ShardRouting.
Also nit -
return Optional.ofNullable(onGoingReplications.getOngoingReplicationTarget(shardRouting.shardId()))
.map(SegmentReplicationTarget::state)
.orElse(null);
* returns SegmentReplicationState of on-going if present or completed segment replication events. | ||
*/ | ||
public SegmentReplicationState getSegmentReplicationState(ShardRouting shardRouting) { | ||
if (getOngoingEventSegmentReplicationState(shardRouting) == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return Optional.ofNullable(getOngoingEventSegmentReplicationState(shardRouting))
.orElseGet(() -> getOngoingEventSegmentReplicationState(shardRouting));
Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Rishikesh1159 <[email protected]>
…h1159/OpenSearch into segment_replication_API
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Rishikesh1159 <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @Rishikesh1159
* Initial Draft for adding segment_replication API Signed-off-by: Rishikesh1159 <[email protected]> * Adding bytes transfered in each segrep events and additional metrics. Signed-off-by: Rishikesh1159 <[email protected]> * Fix broken tests. Signed-off-by: Rishikesh1159 <[email protected]> * Fix compile errors Signed-off-by: Rishikesh1159 <[email protected]> * Adding Tests and gating logic behind feature flag. Signed-off-by: Rishikesh1159 <[email protected]> * Add java docs and enable query parameter detailed. Signed-off-by: Rishikesh1159 <[email protected]> * Add temporary documentation URL Signed-off-by: Rishikesh1159 <[email protected]> * Fixing failing tests. Signed-off-by: Rishikesh1159 <[email protected]> * Spotless Apply. Signed-off-by: Rishikesh1159 <[email protected]> * Fix media type copile check. Signed-off-by: Rishikesh1159 <[email protected]> * Revert previous changes and fix failing tests. Signed-off-by: Rishikesh1159 <[email protected]> * Apply spotless check. Signed-off-by: Rishikesh1159 <[email protected]> * Refactoring call to segmentreplicationstate. Signed-off-by: Rishikesh1159 <[email protected]> * spotless check Signed-off-by: Rishikesh1159 <[email protected]> * Changing invokation of segment replication shard and filtering API response by shard id Signed-off-by: Rishikesh1159 <[email protected]> * disable feature flag by default. Signed-off-by: Rishikesh1159 <[email protected]> * Apply spotless Signed-off-by: Rishikesh1159 <[email protected]> * Address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> * Fix gradle check failures Signed-off-by: Rishikesh1159 <[email protected]> * fix failing testSegment_ReplicationActionAction() Signed-off-by: Rishikesh1159 <[email protected]> * Exclude empty segment replication events in API response. Signed-off-by: Rishikesh1159 <[email protected]> * Apply spotless. Signed-off-by: Rishikesh1159 <[email protected]> * Address PR comments and add Integ Tests. Signed-off-by: Rishikesh1159 <[email protected]> * Fix failing testSegmentReplicationApiResponse(). Signed-off-by: Rishikesh1159 <[email protected]> * Refactoring code. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> (cherry picked from commit e455f56) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
"url":"https://github.com/opensearch-project/documentation-website/issues/2627", | ||
"description":"Returns information about both on-going and latest completed Segment Replication events" | ||
}, | ||
"stability":"stable", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rishikesh1159 Should probably make this experimental
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @andrross for catching this. Yes I missed this, it should be experimental
. Let me make a PR to change this.
…tion API (#6244) * [Segment Replication] Implementing cat/segment_replication API (#5718) * Initial Draft for adding segment_replication API Signed-off-by: Rishikesh1159 <[email protected]> * Adding bytes transfered in each segrep events and additional metrics. Signed-off-by: Rishikesh1159 <[email protected]> * Fix broken tests. Signed-off-by: Rishikesh1159 <[email protected]> * Fix compile errors Signed-off-by: Rishikesh1159 <[email protected]> * Adding Tests and gating logic behind feature flag. Signed-off-by: Rishikesh1159 <[email protected]> * Add java docs and enable query parameter detailed. Signed-off-by: Rishikesh1159 <[email protected]> * Add temporary documentation URL Signed-off-by: Rishikesh1159 <[email protected]> * Fixing failing tests. Signed-off-by: Rishikesh1159 <[email protected]> * Spotless Apply. Signed-off-by: Rishikesh1159 <[email protected]> * Fix media type copile check. Signed-off-by: Rishikesh1159 <[email protected]> * Revert previous changes and fix failing tests. Signed-off-by: Rishikesh1159 <[email protected]> * Apply spotless check. Signed-off-by: Rishikesh1159 <[email protected]> * Refactoring call to segmentreplicationstate. Signed-off-by: Rishikesh1159 <[email protected]> * spotless check Signed-off-by: Rishikesh1159 <[email protected]> * Changing invokation of segment replication shard and filtering API response by shard id Signed-off-by: Rishikesh1159 <[email protected]> * disable feature flag by default. Signed-off-by: Rishikesh1159 <[email protected]> * Apply spotless Signed-off-by: Rishikesh1159 <[email protected]> * Address comments on PR. Signed-off-by: Rishikesh1159 <[email protected]> * Fix gradle check failures Signed-off-by: Rishikesh1159 <[email protected]> * fix failing testSegment_ReplicationActionAction() Signed-off-by: Rishikesh1159 <[email protected]> * Exclude empty segment replication events in API response. Signed-off-by: Rishikesh1159 <[email protected]> * Apply spotless. Signed-off-by: Rishikesh1159 <[email protected]> * Address PR comments and add Integ Tests. Signed-off-by: Rishikesh1159 <[email protected]> * Fix failing testSegmentReplicationApiResponse(). Signed-off-by: Rishikesh1159 <[email protected]> * Refactoring code. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> (cherry picked from commit e455f56) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * Fix compile error. Signed-off-by: Rishikesh1159 <[email protected]> * Fix flaky Tests. Signed-off-by: Rishikesh1159 <[email protected]> * Change stability to experimental in cat.segment_replication.json file. Signed-off-by: Rishikesh1159 <[email protected]> --------- Signed-off-by: Rishikesh1159 <[email protected]> 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> Co-authored-by: Rishikesh1159 <[email protected]>
Signed-off-by: Rishikesh1159 [email protected]
Description
This PR implements segment_replication API to fetch each segment replication event stats.
This PR implements first two points in comment.
-> This API is built by taking reference of
_cat/recovery
API as many components of segment replication and recovery process are same.Overview:
The purpose of the this API is to return metric information about ongoing and completed segment replication events on replica shards. This API returns metric per shard level and this API should only be called on Indices with segment replication enabled.
Paths:
GET/_cat/segment_replication
GET/_cat/segment_replication/<index>
If you want to get information for more than one index, separate the indices with commas:
GET/_cat/segment_replication/index1,index2,index3
Description:
→ cat segment_replication API returns metric information about ongoing and completed segment replication events.
→ Segment Replication is a process of copying segment files from primary shard to replica shards. When primary sends checkpoint to replica shards on a refresh, a new segment replication event is triggered on replica shards.
→ Segment Replication event occurs on following processes:
Query Parameters:
(Optional, Boolean) If true, the response only includes ongoing segment replications. Defaults to false.
(Optional, string) If true, the response only includes ongoing segment replications. Defaults to false.
(Optional, string) Comma-separated list of shards to display.
(Optional, string) Short version of the HTTP accept header. Valid values include JSON, YAML, etc.
(Optional, string) Comma-separated list of column names to display.
(Optional, Boolean) If true, the response includes help information. Defaults to false.
(Optional, string) Comma-separated list or wildcard expression of index names used to limit the request.
(Optional) Unit used to display time values. milliseconds by default.
(Optional, Boolean) If true, the response includes column headings. Defaults to false.
Metric Fields:
default=true
default=true
default=true
default=true
All metrics mentioned below will present in response only when query parameter
detailed=true
Example Response of API:
→ Sample response with no ongoing segment replication events:
→ Sample response with query parameter shards=0, which limits response to only specific shards with ID as 0:
→ Sample response with query parameter detailed=true, which gives more detailed information each stage of segment replication event:
Issues Resolved
Part of #4554
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.