Add Remote Reindex SPI extension #547

soosinha · 2021-04-14T10:56:03Z

Description

This change extends the remote reindex SPI to allow adding a custom interceptor.
This interceptor can be plugged in to perform any processing on the request or response like signing the request using IAM.

Issues Resolved

closes: #511

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.

odfe-release-bot · 2021-04-14T11:00:36Z

✅ DCO Check Passed c7a33e8ad58caf4e320e640494607541f1de458b

odfe-release-bot · 2021-04-14T11:00:36Z

✅ Gradle Wrapper Validation success c7a33e8ad58caf4e320e640494607541f1de458b

odfe-release-bot · 2021-04-14T11:06:41Z

✅ Gradle Precommit success c7a33e8ad58caf4e320e640494607541f1de458b

nknize · 2021-04-15T04:44:29Z

start gradle check

odfe-release-bot · 2021-04-15T05:11:52Z

✅ Gradle Check success c7a33e8ad58caf4e320e640494607541f1de458b
Log 81

Reports 81

peterzhuamazon · 2021-04-15T16:22:11Z

start dco check
start wrapper validation
start gradle precommit

odfe-release-bot · 2021-04-15T16:24:35Z

✅ Gradle Wrapper Validation success c7a33e8ad58caf4e320e640494607541f1de458b

odfe-release-bot · 2021-04-15T16:25:52Z

✅ DCO Check Passed c7a33e8ad58caf4e320e640494607541f1de458b

odfe-release-bot · 2021-04-15T16:39:34Z

✅ Gradle Precommit success c7a33e8ad58caf4e320e640494607541f1de458b

saikaranam-amazon · 2021-04-23T09:17:46Z

modules/reindex/src/main/java/org/opensearch/index/reindex/spi/RemoteReindexExtension.java

+import org.opensearch.index.reindex.BulkByScrollResponse;
+import org.opensearch.index.reindex.ReindexRequest;
+
+public interface RemoteReindexExtension {


could we add additional tests using this extension?

There is no implementation that is added here. Only new interface has been added. Some of the methods have new parameters added to them which should be covered by the existing unit tests.

getsaurabh02 · 2021-04-27T19:01:35Z

modules/reindex/src/main/java/org/opensearch/index/reindex/spi/RemoteReindexExtension.java

+import org.opensearch.index.reindex.BulkByScrollResponse;
+import org.opensearch.index.reindex.ReindexRequest;
+
+public interface RemoteReindexExtension {


Shall we add some java doc here to clarify the intent of this extension point.

Added javadoc

getsaurabh02 · 2021-04-27T19:12:42Z

modules/reindex/src/main/java/org/opensearch/index/reindex/Reindexer.java

+        if (remoteExtension.isPresent()) {
+            return remoteExtension.get().getRemoteReindexActionListener(listener, reindexRequest);
+        }
+        logger.error("Not able to load remote reindex metrics listener");


This error message seems confusing, in case there are no actual extensions present to the Reindexer plugin.

Changed the message

getsaurabh02 · 2021-04-27T19:14:59Z

modules/reindex/src/main/java/org/opensearch/index/reindex/spi/RemoteReindexExtension.java

+     * Get a wrapper of ActionListener which is used to collect remote reindex metrics
+     * @return ActionListener wrapper implementation.


Should we keep the comment more generic, as action listeners could serve more purpose than just the metrics.

Made the comment generic

odfe-release-bot · 2021-04-28T11:14:01Z

✅ Gradle Wrapper Validation success df5e90161387b43bc4644ec24740c0d90c4cf3ee

odfe-release-bot · 2021-04-28T11:14:01Z

✅ DCO Check Passed df5e90161387b43bc4644ec24740c0d90c4cf3ee

odfe-release-bot · 2021-04-28T11:20:29Z

✅ Gradle Precommit success df5e90161387b43bc4644ec24740c0d90c4cf3ee

harold-wang · 2021-05-03T14:42:42Z

start gradle check

opensearch-ci-bot · 2021-05-03T14:55:15Z

❌ Gradle Check failure df5e90161387b43bc4644ec24740c0d90c4cf3ee
Log 152

Reports 152

opensearch-ci-bot · 2021-05-03T16:13:16Z

✅ DCO Check Passed c0836948b02743ee598b7af0c5173a127f9634f8

opensearch-ci-bot · 2021-05-03T16:13:54Z

✅ Gradle Wrapper Validation success c0836948b02743ee598b7af0c5173a127f9634f8

opensearch-ci-bot · 2021-05-03T16:22:04Z

✅ Gradle Precommit success c0836948b02743ee598b7af0c5173a127f9634f8

harold-wang · 2021-05-03T17:07:16Z

start gradle check

opensearch-ci-bot · 2021-05-03T17:28:24Z

✅ Gradle Check success c0836948b02743ee598b7af0c5173a127f9634f8
Log 153

Reports 153

adnapibar

LGTM

adnapibar · 2021-05-05T22:52:02Z

modules/reindex/src/main/java/org/opensearch/index/reindex/ReindexPlugin.java

+        List<RemoteReindexExtension> remoteReindexExtensionList = new ArrayList<>();
+        iterable.forEach(remoteReindexExtensionList::add);
+        if (remoteReindexExtensionList.isEmpty()) {
+            logger.error("Unable to find any implementation for RemoteReindexExtension");


Why are we logging an error here?

Changed to info log.

This change extends the remote reindex SPI to allow adding a custom interceptor. This interceptor can be plugged in to perform any processing on the request or response. Signed-off-by: Sooraj Sinha <soosinha@amazon.com>

opensearch-ci-bot · 2021-05-06T04:26:23Z

✅ Gradle Wrapper Validation success 094e16e

opensearch-ci-bot · 2021-05-06T04:26:51Z

✅ DCO Check Passed 094e16e

opensearch-ci-bot · 2021-05-06T04:34:24Z

✅ Gradle Precommit success 094e16e

adnapibar · 2021-05-06T15:11:33Z

start gradle check

opensearch-ci-bot · 2021-05-06T15:32:52Z

✅ Gradle Check success 094e16e
Log 164

Reports 164

nknize · 2021-05-06T18:07:04Z

What version are you targeting? Do we want this in 1.0.0? If so you'll need to open a separate backport PR.

soosinha · 2021-05-06T18:30:43Z

@nknize This is targetted for 1.0.0. Can you let me know the process for a backport PR ? What version would it be backported to as 1.0.0 is not yet released ?

This change extends the remote reindex SPI to allow adding a custom interceptor. This interceptor can be plugged in to perform any processing on the request or response. Signed-off-by: Sooraj Sinha <soosinha@amazon.com> Co-authored-by: Sooraj Sinha <81695996+soosinha@users.noreply.github.com>

Signed-off-by: Abbas Hussain <abbas_10690@yahoo.com>

saikaranam-amazon reviewed Apr 23, 2021

View reviewed changes

saikaranam-amazon approved these changes Apr 23, 2021

View reviewed changes

getsaurabh02 reviewed Apr 27, 2021

View reviewed changes

soosinha force-pushed the reindex_spi_extension branch from c7a33e8 to df5e901 Compare April 28, 2021 11:09

getsaurabh02 approved these changes May 3, 2021

View reviewed changes

soosinha force-pushed the reindex_spi_extension branch from df5e901 to c083694 Compare May 3, 2021 16:11

adnapibar approved these changes May 5, 2021

View reviewed changes

Add Remote Reindex SPI extension

094e16e

This change extends the remote reindex SPI to allow adding a custom interceptor. This interceptor can be plugged in to perform any processing on the request or response. Signed-off-by: Sooraj Sinha <soosinha@amazon.com>

soosinha force-pushed the reindex_spi_extension branch from c083694 to 094e16e Compare May 6, 2021 04:24

adnapibar merged commit 181ee8a into opensearch-project:main May 6, 2021

nknize added the v2.0.0 Version 2.0.0 label May 6, 2021

nknize added the enhancement Enhancement or improvement to existing feature or request label May 6, 2021

abbashus added the v1.0.0 Version 1.0.0 label May 21, 2021

abbashus mentioned this pull request May 21, 2021

[1.x] Add Remote Reindex SPI extension (#547) #756

Merged

This was referenced Jun 24, 2021

Rebuild Rest client after setting interceptor #876

Merged

[BUG] Custom interceptor not attached to the Remote reindex Rest client #890

Closed

This was referenced Jul 5, 2021

[1.0] Pass interceptor to super constructor (#876) #936

Closed

[1.x] Pass interceptor to super constructor (#876) #937

Merged

abbashus mentioned this pull request Jul 12, 2021

Update release note for GA 1.0 #953

Merged

5 tasks

abbashus added a commit that referenced this pull request Jul 12, 2021

Update release note for GA 1.0 with new commits and removes #547 (#953)

6252370

Signed-off-by: Abbas Hussain <abbas_10690@yahoo.com>

		* Get a wrapper of ActionListener which is used to collect remote reindex metrics
		* @return ActionListener wrapper implementation.

Add Remote Reindex SPI extension #547

Add Remote Reindex SPI extension #547

Conversation

soosinha commented Apr 14, 2021

Description

Issues Resolved

Check List

odfe-release-bot commented Apr 14, 2021

odfe-release-bot commented Apr 14, 2021

odfe-release-bot commented Apr 14, 2021

nknize commented Apr 15, 2021

odfe-release-bot commented Apr 15, 2021

peterzhuamazon commented Apr 15, 2021

odfe-release-bot commented Apr 15, 2021

odfe-release-bot commented Apr 15, 2021

odfe-release-bot commented Apr 15, 2021

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

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

odfe-release-bot commented Apr 28, 2021

odfe-release-bot commented Apr 28, 2021

odfe-release-bot commented Apr 28, 2021

harold-wang commented May 3, 2021

opensearch-ci-bot commented May 3, 2021

opensearch-ci-bot commented May 3, 2021

opensearch-ci-bot commented May 3, 2021

opensearch-ci-bot commented May 3, 2021

harold-wang commented May 3, 2021

opensearch-ci-bot commented May 3, 2021

adnapibar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

opensearch-ci-bot commented May 6, 2021

opensearch-ci-bot commented May 6, 2021

opensearch-ci-bot commented May 6, 2021

adnapibar commented May 6, 2021 • edited Loading

opensearch-ci-bot commented May 6, 2021

nknize commented May 6, 2021

soosinha commented May 6, 2021

adnapibar commented May 6, 2021 •

edited

Loading