Move common constants to Remote constants class, simplify RemoteBuild…

…Request using serialization logic (XContent)

Signed-off-by: owenhalpert <ohalpert@gmail.com>

src/main/java/org/opensearch/knn/common/KNNConstants.java

@@ -165,25 +165,4 @@ public class KNNConstants {
     public static final String DERIVED_VECTOR_FIELD_ATTRIBUTE_KEY = "knn-derived-source-enabled";
     public static final String DERIVED_VECTOR_FIELD_ATTRIBUTE_TRUE_VALUE = "true";
     public static final String DERIVED_VECTOR_FIELD_ATTRIBUTE_FALSE_VALUE = "false";
-
-    // Remote build constants
-    public static final String BUILD_ENDPOINT = "/_build";
-    public static final String STATUS_ENDPOINT = "/_status";
-    public static final String S3 = "s3";
-    public static final String BUCKET = "bucket";
-    // Build request keys
-    public static final String ALGORITHM = "algorithm";
-    public static final String ALGORITHM_PARAMETERS = "algorithm_parameters";
-    public static final String INDEX_PARAMETERS = "index_parameters";
-    public static final String DOC_COUNT = "doc_count";
-    public static final String TENANT_ID = "tenant_id";
-    public static final String DOC_ID_PATH = "doc_id_path";
-    public static final String VECTOR_PATH = "vector_path";
-    public static final String CONTAINER_NAME = "container_name";
-    public static final String REPOSITORY_TYPE = "repository_type";
-    // Server responses
-    public static final String JOB_ID = "job_id";
-    public static final String TASK_STATUS = "task_status";
-    public static final String INDEX_PATH = "index_path";
-    public static final String ERROR_MESSAGE = "error_message";
 }

src/main/java/org/opensearch/knn/index/codec/nativeindex/remote/DefaultVectorRepositoryAccessor.java

@@ -33,10 +33,10 @@
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Supplier;
 
-import static org.opensearch.knn.index.codec.nativeindex.remote.RemoteIndexBuildStrategy.DOC_ID_FILE_EXTENSION;
-import static org.opensearch.knn.index.codec.nativeindex.remote.RemoteIndexBuildStrategy.VECTORS_PATH;
-import static org.opensearch.knn.index.codec.nativeindex.remote.RemoteIndexBuildStrategy.VECTOR_BLOB_FILE_EXTENSION;
 import static org.opensearch.knn.index.codec.util.KNNCodecUtil.initializeVectorValues;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.DOC_ID_FILE_EXTENSION;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.VECTORS_PATH;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.VECTOR_BLOB_FILE_EXTENSION;
 
 @Log4j2
 @AllArgsConstructor

src/main/java/org/opensearch/knn/index/codec/nativeindex/remote/RemoteIndexBuildStrategy.java

@@ -13,9 +13,7 @@
 import org.opensearch.knn.index.KNNSettings;
 import org.opensearch.knn.index.codec.nativeindex.NativeIndexBuildStrategy;
 import org.opensearch.knn.index.codec.nativeindex.model.BuildIndexParams;
-import org.opensearch.knn.index.remote.HTTPRemoteBuildRequest;
 import org.opensearch.knn.index.remote.RemoteBuildRequest;
-import org.opensearch.knn.index.remote.RemoteBuildRequestBuilder;
 import org.opensearch.knn.index.remote.RemoteBuildResponse;
 import org.opensearch.knn.index.remote.RemoteIndexClient;
 import org.opensearch.knn.index.remote.RemoteIndexClientFactory;
@@ -43,10 +41,6 @@ public class RemoteIndexBuildStrategy implements NativeIndexBuildStrategy {
     private final NativeIndexBuildStrategy fallbackStrategy;
     private final IndexSettings indexSettings;
 
-    public static final String VECTOR_BLOB_FILE_EXTENSION = ".knnvec";
-    public static final String DOC_ID_FILE_EXTENSION = ".knndid";
-    static final String VECTORS_PATH = "_vectors";
-
     /**
      * Public constructor, intended to be called by {@link org.opensearch.knn.index.codec.nativeindex.NativeIndexBuildStrategyFactory} based in
      * part on the return value from {@link RemoteIndexBuildStrategy#shouldBuildIndexRemotely}
@@ -130,14 +124,8 @@ public void buildAndWriteIndex(BuildIndexParams indexInfo) throws IOException {
             time_in_millis = stopWatch.stop().totalTime().millis();
             log.debug("Repository write took {} ms for vector field [{}]", time_in_millis, indexInfo.getFieldName());
 
-            // TODO future implementations will set the following two params depending on some setting to denote the protocol
-            RemoteIndexClient client = RemoteIndexClientFactory.getRemoteIndexClient(RemoteIndexClientFactory.TYPE_HTTP);
-            RemoteBuildRequest request = RemoteBuildRequestBuilder.builder(HTTPRemoteBuildRequest.class)
-                .indexSettings(indexSettings)
-                .indexInfo(indexInfo)
-                .repositoryMetadata(getRepository().getMetadata())
-                .blobName(blobName)
-                .build();
+            RemoteIndexClient client = RemoteIndexClientFactory.getRemoteIndexClient();
+            RemoteBuildRequest request = new RemoteBuildRequest(indexSettings, indexInfo, getRepository().getMetadata(), blobName);
             stopWatch = new StopWatch().start();
             RemoteBuildResponse remoteBuildResponse = client.submitVectorBuild(request);
             time_in_millis = stopWatch.stop().totalTime().millis();

src/main/java/org/opensearch/knn/index/engine/faiss/Faiss.java

@@ -19,8 +19,6 @@
 import java.util.Map;
 import java.util.function.Function;
 
-import static org.opensearch.knn.common.KNNConstants.ALGORITHM;
-import static org.opensearch.knn.common.KNNConstants.ALGORITHM_PARAMETERS;
 import static org.opensearch.knn.common.KNNConstants.INDEX_DESCRIPTION_PARAMETER;
 import static org.opensearch.knn.common.KNNConstants.METHOD_HNSW;
 import static org.opensearch.knn.common.KNNConstants.METHOD_IVF;
@@ -35,6 +33,8 @@
 import static org.opensearch.knn.common.KNNConstants.NAME;
 import static org.opensearch.knn.common.KNNConstants.PARAMETERS;
 import static org.opensearch.knn.common.KNNConstants.SPACE_TYPE;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.ALGORITHM;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.ALGORITHM_PARAMETERS;
 import static org.opensearch.knn.index.KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_CONSTRUCTION;
 import static org.opensearch.knn.index.KNNSettings.INDEX_KNN_DEFAULT_ALGO_PARAM_EF_SEARCH;
 import static org.opensearch.knn.index.KNNSettings.INDEX_KNN_DEFAULT_SPACE_TYPE;

src/main/java/org/opensearch/knn/index/remote/KNNRemoteConstants.java

@@ -0,0 +1,27 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.knn.index.remote;
+
+// Public class to define the constants used by Remote Index Build in 2 or more classes.
+public class KNNRemoteConstants {
+    // Repository filepath constants
+    public static final String VECTOR_BLOB_FILE_EXTENSION = ".knnvec";
+    public static final String DOC_ID_FILE_EXTENSION = ".knndid";
+    public static final String VECTORS_PATH = "_vectors";
+
+    // Repository-S3
+    public static final String S3 = "s3";
+    public static final String BUCKET = "bucket";
+
+    // Build request keys
+    public static final String ALGORITHM = "algorithm";
+    public static final String ALGORITHM_PARAMETERS = "algorithm_parameters";
+    public static final String INDEX_PARAMETERS = "index_parameters";
+
+    // HTTP implementation
+    public static final String BUILD_ENDPOINT = "/_build";
+    public static final String STATUS_ENDPOINT = "/_status";
+}

src/main/java/org/opensearch/knn/index/remote/RemoteBuildRequest.java

@@ -8,23 +8,38 @@
 import lombok.Getter;
 import org.opensearch.cluster.ClusterName;
 import org.opensearch.cluster.metadata.RepositoryMetadata;
+import org.opensearch.core.xcontent.ToXContentObject;
+import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.index.IndexSettings;
 import org.opensearch.knn.index.codec.nativeindex.model.BuildIndexParams;
-import org.opensearch.knn.index.codec.nativeindex.remote.RemoteIndexBuildStrategy;
 import org.opensearch.knn.index.codec.util.KNNCodecUtil;
 import org.opensearch.knn.index.vectorvalues.KNNVectorValues;
 
 import java.io.IOException;
 import java.util.Map;
 
-import static org.opensearch.knn.common.KNNConstants.BUCKET;
-import static org.opensearch.knn.common.KNNConstants.S3;
+import static org.opensearch.knn.common.KNNConstants.DIMENSION;
+import static org.opensearch.knn.common.KNNConstants.KNN_ENGINE;
+import static org.opensearch.knn.common.KNNConstants.VECTOR_DATA_TYPE_FIELD;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.BUCKET;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.DOC_ID_FILE_EXTENSION;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.INDEX_PARAMETERS;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.S3;
+import static org.opensearch.knn.index.remote.KNNRemoteConstants.VECTOR_BLOB_FILE_EXTENSION;
 
 /**
- * Abstract base class for Remote Build Requests.
+ * Request object for sending build requests to the remote build service, encapsulating all the required parameters
+ * in a generic XContent format.
  */
 @Getter
-public abstract class RemoteBuildRequest {
+public class RemoteBuildRequest implements ToXContentObject {
+    private final String DOC_COUNT = "doc_count";
+    private final String TENANT_ID = "tenant_id";
+    private final String DOC_ID_PATH = "doc_id_path";
+    public final String VECTOR_PATH = "vector_path";
+    public final String CONTAINER_NAME = "container_name";
+    public final String REPOSITORY_TYPE = "repository_type";
+
     protected String repositoryType;
     protected String containerName;
     protected String vectorPath;
@@ -36,6 +51,15 @@ public abstract class RemoteBuildRequest {
     protected String engine;
     protected Map<String, Object> indexParameters;
 
+    /**
+     * Constructor for RemoteBuildRequest.
+     *
+     * @param indexSettings IndexSettings object
+     * @param indexInfo BuildIndexParams object
+     * @param repositoryMetadata RepositoryMetadata object
+     * @param blobName Name of the blob
+     * @throws IOException if an I/O error occurs
+     */
     public RemoteBuildRequest(
         IndexSettings indexSettings,
         BuildIndexParams indexInfo,
@@ -60,8 +84,8 @@ public RemoteBuildRequest(
 
         this.repositoryType = repositoryType;
         this.containerName = containerName;
-        this.vectorPath = blobName + RemoteIndexBuildStrategy.VECTOR_BLOB_FILE_EXTENSION;
-        this.docIdPath = blobName + RemoteIndexBuildStrategy.DOC_ID_FILE_EXTENSION;
+        this.vectorPath = blobName + VECTOR_BLOB_FILE_EXTENSION;
+        this.docIdPath = blobName + DOC_ID_FILE_EXTENSION;
         this.tenantId = indexSettings.getSettings().get(ClusterName.CLUSTER_NAME_SETTING.getKey());
         this.dimension = vectorValues.dimension();
         this.docCount = indexInfo.getTotalLiveDocs();
@@ -70,8 +94,20 @@ public RemoteBuildRequest(
         this.indexParameters = indexParameters;
     }
 
-    /**
-     * Convert the request to JSON format.
-     */
-    public abstract String toJson() throws IOException;
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
+        builder.field(REPOSITORY_TYPE, repositoryType);
+        builder.field(CONTAINER_NAME, containerName);
+        builder.field(VECTOR_PATH, vectorPath);
+        builder.field(DOC_ID_PATH, docIdPath);
+        builder.field(TENANT_ID, tenantId);
+        builder.field(DIMENSION, dimension);
+        builder.field(DOC_COUNT, docCount);
+        builder.field(VECTOR_DATA_TYPE_FIELD, vectorDataType);
+        builder.field(KNN_ENGINE, engine);
+        builder.field(INDEX_PARAMETERS, indexParameters);
+        builder.endObject();
+        return builder;
+    }
 }

src/main/java/org/opensearch/knn/index/remote/RemoteBuildResponse.java

@@ -5,9 +5,45 @@
 
 package org.opensearch.knn.index.remote;
 
+import lombok.Builder;
+import lombok.Value;
+import org.apache.commons.lang.StringUtils;
+import org.opensearch.core.ParseField;
+import org.opensearch.core.xcontent.XContentParser;
+
+import java.io.IOException;
+
 /**
- * Remote build response. Currently, this just contains the jobId from the server.
- * In the future, this may be an interface if different clients expect different responses.
+ * Response from the remote index build service. This class is used to parse the response from the remote index build service.
  */
-public record RemoteBuildResponse(String jobId) {
+@Value
+@Builder
+public class RemoteBuildResponse {
+    private static final String JOB_ID_FIELD = "job_id";
+    private static final ParseField JOB_ID = new ParseField(JOB_ID_FIELD);
+    String jobId;
+
+    public static RemoteBuildResponse fromXContent(XContentParser parser) throws IOException {
+        final RemoteBuildResponseBuilder builder = new RemoteBuildResponseBuilder();
+        XContentParser.Token token = parser.nextToken();
+        if (token != XContentParser.Token.START_OBJECT) {
+            throw new IOException("Invalid response format, was expecting a " + XContentParser.Token.START_OBJECT);
+        }
+        String currentFieldName = null;
+        while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
+            if (token == XContentParser.Token.FIELD_NAME) {
+                currentFieldName = parser.currentName();
+            } else if (token.isValue()) {
+                if (JOB_ID.match(currentFieldName, parser.getDeprecationHandler())) {
+                    builder.jobId(parser.text());
+                } else {
+                    throw new IOException("Invalid response format, unknown field: " + currentFieldName);
+                }
+            }
+        }
+        if (StringUtils.isBlank(builder.jobId)) {
+            throw new IOException("Invalid response format, missing + " + JOB_ID_FIELD);
+        }
+        return builder.build();
+    }
 }