Add changes to Point in time segments API service layer #4105

bharath-techie · 2022-08-03T09:46:18Z

Signed-off-by: Bharathwaj G bharath78910@gmail.com

Description

This contains service layer changes for retrieving segments information of PITs which provides information about disk utilization of PIT

Issues Resolved

#1147

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

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.

github-actions · 2022-08-03T10:08:37Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1344/
CommitID: 760c517589c2d4996d4784a64a606d3de2a15254

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

github-actions · 2022-08-03T12:20:44Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1351/
CommitID: 42f5f10

pranikum · 2022-08-03T15:16:41Z

server/src/main/java/org/opensearch/action/admin/indices/segments/PitSegmentsRequest.java

+        return pitIds;
+    }
+
+    public void setPitIds(Collection<String> pitIds) {


Any reason for having setter. We can have it initialised during construction.

Yeah in transport layer, if no pit ids are passed, we get all PITs and set it in request.

pranikum · 2022-08-03T15:17:40Z

server/src/main/java/org/opensearch/action/admin/indices/segments/PitSegmentsRequest.java

+
+    }
+
+    public Collection<String> getPitIds() {


UnmodifiableList ? Also i don't see anywhere we are using it apart from List type. We can use List itself.

pranikum · 2022-08-03T15:21:00Z

server/src/main/java/org/opensearch/action/admin/indices/segments/PitSegmentsRequest.java

+ */
+public class PitSegmentsRequest extends BroadcastRequest<PitSegmentsRequest> {
+
+    protected boolean verbose = false;


Why protected if we already have setter and getter public methods.
Since default value is false. So don't need to mention it specifically.

pranikum · 2022-08-03T15:21:29Z

server/src/main/java/org/opensearch/action/admin/indices/segments/PitSegmentsRequest.java

+     * Sets the <code>verbose</code> option.
+     * @see #verbose()
+     */
+    public void verbose(boolean v) {


setVerbose ?

pranikum · 2022-08-03T15:21:40Z

server/src/main/java/org/opensearch/action/admin/indices/segments/PitSegmentsRequest.java

+     * <code>true</code> if detailed information about each segment should be returned,
+     * <code>false</code> otherwise.
+     */
+    public boolean verbose() {


isVerbose for clarity?

github-actions · 2022-08-03T17:04:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1367/
CommitID: 959a28f039b51bdc264394821e2b5428511d8441

...r/src/main/java/org/opensearch/action/admin/indices/segments/TransportPitSegmentsAction.java

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

github-actions · 2022-08-04T05:55:08Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1431/
CommitID: 6d2d1d8495e4994a13b7fb722b214fe27132a547

github-actions · 2022-08-04T06:12:18Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1432/
CommitID: 706375289d2fcaa5f36ced9cfb04c40843e8fe28

github-actions · 2022-08-04T06:15:07Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1433/
CommitID: c29f252077103d9e87e9cefe102345133d5aaaae

pranikum · 2022-08-04T06:20:53Z

server/src/main/java/org/opensearch/search/internal/PitReaderContext.java

+    }
+
+    public List<Segment> getSegments() {
+        return segments;


Collection.unmodifiable

pranikum · 2022-08-04T06:28:52Z

server/src/test/java/org/opensearch/action/search/PitTestsUtil.java

+        IndicesSegmentResponse indicesSegmentResponse = client.execute(PitSegmentsAction.INSTANCE, new PitSegmentsRequest()).actionGet();
+        assertTrue(indicesSegmentResponse.getShardFailures() == null || indicesSegmentResponse.getShardFailures().length == 0);
+        if (isEmpty) {
+            assertTrue(indicesSegmentResponse.getIndices().size() == 0);


we can use assertEquals(indicesSegmentResponse.getIndices().isEmpty(), isEmpty); and avoid if/else block

github-actions · 2022-08-04T06:30:14Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1435/
CommitID: cd5263f53e287c9419e764056f7ca91a257e1c45

pranikum · 2022-08-04T06:30:56Z

server/src/main/java/org/opensearch/action/admin/indices/segments/PitSegmentsRequest.java

+ * Transport request for retrieving PITs segment information
+ */
+public class PitSegmentsRequest extends BroadcastRequest<PitSegmentsRequest> {
+    boolean verbose = false;


we can simply write private boolean verbose;. Since default initialisation is false for boolean. No need to give package access to variable as we already have public setter.

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

github-actions · 2022-08-04T07:46:00Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1439/
CommitID: babab92

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

sachinpkale · 2022-08-17T03:50:13Z

The build is failing, please fix it.

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

github-actions · 2022-08-17T12:41:39Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1828/
CommitID: 51ef240

Bukhtawar · 2022-08-18T06:13:57Z

...r/src/main/java/org/opensearch/action/admin/indices/segments/TransportPitSegmentsAction.java

+     */
+    @Override
+    protected ShardSegments shardOperation(PitSegmentsRequest request, ShardRouting shardRouting) {
+        if (shardRouting instanceof PitAwareShardRouting) {


Should this be an assertion instead of an if condition

Bukhtawar · 2022-08-18T06:23:18Z

...r/src/main/java/org/opensearch/action/admin/indices/segments/TransportPitSegmentsAction.java

+                .get(shardRouting.shardId());
+            PitReaderContext pitReaderContext = searchService.getPitReaderContext(searchContextIdForNode.getSearchContextId());
+            if (pitReaderContext == null) {
+                return new ShardSegments(shardRouting, new ArrayList<>());


Collections.emptyList() instead?

Bukhtawar · 2022-08-18T06:23:44Z

...r/src/main/java/org/opensearch/action/admin/indices/segments/TransportPitSegmentsAction.java

+        } else {
+            throw new IllegalArgumentException("Shard routing is not of PitAwareShardRouting type");
+        }


lets remove and replace with assertion?

github-actions · 2022-08-18T07:48:16Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1875/
CommitID: bdc97fb8aa0b74dded9d5430547a3f36e36ec889

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

github-actions · 2022-08-18T08:20:15Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1878/
CommitID: 903588e

… into pitsegments

github-actions · 2022-08-18T12:38:04Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1884/
CommitID: aa04c05

bharath-techie · 2022-08-19T16:59:16Z

@reta @Bukhtawar I've addressed comments, if changes look good can you please approve / merge.

Bukhtawar · 2022-08-19T17:48:56Z

...r/src/main/java/org/opensearch/action/admin/indices/segments/TransportPitSegmentsAction.java

+    @Override
+    protected ClusterBlockException checkGlobalBlock(ClusterState state, PitSegmentsRequest request) {
+        return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_READ);
+    }


should this be READ instead of METADATA_READ?

METADATA_READ is used in index segments as well. Read is used in operations like search, multisearch etc.

Bukhtawar

Left a couple of minor comments. Please review them

… into pitsegments

github-actions · 2022-08-22T11:43:02Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1975/
CommitID: b3a9c875edf983b14129e1d56a21c03e3dad2ec9

github-actions · 2022-08-22T12:12:31Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/1978/
CommitID: f100c7bb4fd34f4390950e70602714d7f4d402d1

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

github-actions · 2022-08-22T13:14:39Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/1983/
CommitID: 78c645b

bharath-techie · 2022-08-22T16:13:59Z

@reta @Bukhtawar can we please merge this ?

… into pitsegments

github-actions · 2022-08-23T05:52:42Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/2013/
CommitID: 5d814ac

bharath-techie requested review from a team and reta as code owners August 3, 2022 09:46

dhruv16dhr mentioned this pull request Aug 3, 2022

[META] Point In Time Search Issue List #3959

Open

47 tasks

pit segments service layer changes

42f5f10

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the pitsegments branch from 760c517 to 42f5f10 Compare August 3, 2022 11:57

pranikum reviewed Aug 3, 2022

View reviewed changes

pranikum reviewed Aug 4, 2022

View reviewed changes

...r/src/main/java/org/opensearch/action/admin/indices/segments/TransportPitSegmentsAction.java Show resolved Hide resolved

Addressing comment

1c702de

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the pitsegments branch 4 times, most recently from c29f252 to cd5263f Compare August 4, 2022 05:54

pranikum reviewed Aug 4, 2022

View reviewed changes

Addressing comment

babab92

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the pitsegments branch from cd5263f to babab92 Compare August 4, 2022 07:33

Addressing comment

adf80d6

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

addressing comments

51ef240

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the pitsegments branch from 184e206 to 51ef240 Compare August 17, 2022 12:17

Bukhtawar reviewed Aug 18, 2022

View reviewed changes

Addressing comments

903588e

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the pitsegments branch from bdc97fb to 903588e Compare August 18, 2022 07:58

Merge branch 'main' of https://github.com/opensearch-project/OpenSearch…

aa04c05

… into pitsegments

Bukhtawar reviewed Aug 19, 2022

View reviewed changes

Bukhtawar approved these changes Aug 19, 2022

View reviewed changes

Merge branch 'main' of https://github.com/opensearch-project/OpenSearch…

611ed25

… into pitsegments

bharath-techie force-pushed the pitsegments branch from b3a9c87 to f100c7b Compare August 22, 2022 11:48

reta approved these changes Aug 22, 2022

View reviewed changes

Adding '_all' as option to get all segments

78c645b

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the pitsegments branch from f100c7b to 78c645b Compare August 22, 2022 12:51

Merge branch 'main' of https://github.com/opensearch-project/OpenSearch…

5d814ac

… into pitsegments

reta merged commit 9d6255c into opensearch-project:main Aug 23, 2022

bharath-techie mentioned this pull request Sep 5, 2022

[Backport 2.x] [Point in time] Backport point in time changes #4406

Closed

6 tasks

bharath-techie mentioned this pull request Sep 28, 2022

[Backport 2.x] [Point in time] Backport point in time changes #4616

Merged

6 tasks

Add changes to Point in time segments API service layer #4105

Add changes to Point in time segments API service layer #4105

Conversation

bharath-techie commented Aug 3, 2022 • edited Loading

Description

Issues Resolved

Check List

github-actions bot commented Aug 3, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 3, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pranikum Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

pranikum Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

pranikum Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

pranikum Aug 3, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Aug 3, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

pranikum Aug 4, 2022 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Aug 4, 2022

Gradle Check (Jenkins) Run Completed with:

sachinpkale commented Aug 17, 2022

github-actions bot commented Aug 17, 2022

Gradle Check (Jenkins) Run Completed with:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 18, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 18, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 18, 2022

Gradle Check (Jenkins) Run Completed with:

bharath-techie commented Aug 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bukhtawar left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 22, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 22, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 22, 2022

Gradle Check (Jenkins) Run Completed with:

bharath-techie commented Aug 22, 2022

github-actions bot commented Aug 23, 2022

Gradle Check (Jenkins) Run Completed with:

bharath-techie commented Aug 3, 2022 •

edited

Loading

pranikum Aug 3, 2022 •

edited

Loading

pranikum Aug 3, 2022 •

edited

Loading

pranikum Aug 3, 2022 •

edited

Loading

pranikum Aug 3, 2022 •

edited

Loading

pranikum Aug 4, 2022 •

edited

Loading