add multitenant support for search workloads #13713

kaushalmahi12 · 2024-05-16T17:37:44Z

Description

This PR add support to add labels to support multitenancy in opensearch. These labels can be consumed by any feature e,g; Query Sandboxing, Query Insights, Slow logs etc;

Sample search request body to pass these labels

curl -X GET "localhost:9200/my-index-*/_search?size=1000&pretty" -H 'Content-Type: application/json' -d '{
  "query" : {
  },
  "labels": {
    "tenant": "analytics"
  }
}'

Related Issues

#12342

Check List

New functionality includes testing.
- All tests pass
[] New functionality has been documented.
- New functionality has javadoc added
API changes companion pull request created.
Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)
Public documentation issue/PR created

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.

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

…odes Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

github-actions · 2024-05-16T18:26:39Z

❌ Gradle check result for 675a105: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

github-actions · 2024-05-16T18:32:25Z

❌ Gradle check result for c8f262b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

prudhvigodithi · 2024-05-24T01:28:00Z

Hey, If the Gradle check fails with error No such property: post_merge_action, please merge/rebase the PR from upstream to ensure the post_merge_action field is part of the Gradle check setup. More details in the PR 13786.

sgup432 · 2024-05-22T21:30:33Z

server/src/main/java/org/opensearch/search/SearchService.java

+        String tenant = NOT_PROVIDED;
+        if (request.source() != null && request.source().multiTenantLabels() != null) {
+            tenant = (String) request.source().multiTenantLabels().get(MultiTenantLabel.TENANT.name());
+        }


[NIT] Can be done inline like below:

String tenant = Optional.ofNullable(request.source()) .map(source -> source.multiTenantLabels()) .map(tenantLabels -> tenantLabels.get(MultiTenantLabel.TENANT.name()) .orElse(NOT_PROVIDED);

sgup432 · 2024-05-22T21:37:15Z

server/src/main/java/org/opensearch/search/SearchService.java

@@ -568,6 +569,7 @@ public void executeQueryPhase(
        assert request.canReturnNullResponseIfMatchNoDocs() == false || request.numberOfShards() > 1
            : "empty responses require more than one shard";
        final IndexShard shard = getShard(request);
+        setTenantInTask(task, request);


Should we not also instrument this tenant logic in task for other phases like fetch etc?
If yes, we should ideally populate this tenant info from coordinator level itself so that all tasks automatically have this info?

We do populate the tenancy info at the coordinator level but it would not suffice because of following reasons

Shard level tasks are created at the data node when we receive the request. TaskManager creates the task using a TaskAwareRequest. https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/transport/RequestHandlerRegistry.java#L91

For some use cases We might want to perform the tagging at shard level. Specially if the index is a tenant.

Okay. But should we also instrument this tenant logic for other phases as well like fetch etc?

sgup432 · 2024-05-22T21:45:48Z

server/src/main/java/org/opensearch/action/search/TransportSearchAction.java

@@ -1104,6 +1106,15 @@ private void executeSearch(
            concreteLocalIndices,
            localShardIterators.size() + remoteShardIterators.size()
        );
+
+        // Set tenant for this request in the task for tracking the tasks across tenants
+        String tenant = NOT_PROVIDED;


Considering this logic can also be extended to other places like indexing etc, defining this here doesn't make sense.
Either

Define a global string/enum for this in tenant related class
OR

Leave it tenant name as null, make it Optional and handle it accordingly?

Thanks! good catch. I have defined a global string just forgot to use that here.

sgup432 · 2024-05-28T07:47:37Z

server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java

@@ -297,6 +300,10 @@ public SearchSourceBuilder(StreamInput in) throws IOException {
                derivedFields = in.readList(DerivedField::new);
            }
        }
+
+        if (in.getVersion().onOrAfter(Version.V_2_14_0)) {


You will need to define this to Version.V_3_0_0 initially in main branch, after backporting to 2.x, you will then need to change it to Version.V_2_14_0

sgup432 · 2024-05-28T07:48:12Z

server/src/main/java/org/opensearch/search/builder/SearchSourceBuilder.java

@@ -223,6 +225,7 @@ public static HighlightBuilder highlight() {
    private PointInTimeBuilder pointInTimeBuilder = null;

    private Map<String, Object> searchPipelineSource = null;
+    private Map<String, Object> multiTenantLabels = new HashMap<>();


Why not just define it to Map<String, String>?

Keeping it generic if want to provide some complex objects as part of tenancy definition.

sgup432 · 2024-05-28T07:59:24Z

server/src/main/java/org/opensearch/tasks/ResourceLimitGroupTask.java

+/**
+ * Tasks which should be grouped
+ */
+public interface ResourceLimitGroupTask {


I think we are essentially grouping by "tenants" which internally have resource limits. So this naming doesn't look right to me.

The naming part we are still discussing.

kaushalmahi12 added 13 commits May 15, 2024 20:28

add sandbox service

661e4f8

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add resourceLimitGroupId propagation logic from coordinator to data n…

c8cde73

…odes Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add sandbox schema

7bf8b57

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add resourceLimitGroupTests

bc6f2b9

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add resourceLimitGroupMetadata tests

330ca66

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

run spotlessApply

f94c4f9

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add mode field in ResourceLimitGroup schema

823e9a6

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

fix breaking testcases

01fda32

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add task cancellation skeleton

da39ae9

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add multitenant labels in searchSource builder

db05a10

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add tenant label info

4c6b46a

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

add multitenant label tests

92f2620

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

apply spotless task

675a105

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

kaushalmahi12 requested review from anasalkouz, andrross, Bukhtawar, CEHENKLE, dblock, dbwiddis, dreamer-89, gbbafna, kotwanikunal, mch2, msfroh, nknize, owaiskazi19, reta, Rishikesh1159, sachinpkale and saratvemulapalli as code owners May 16, 2024 17:37

kaushalmahi12 requested review from shwetathareja, sohami, tlfeng and VachaShah as code owners May 16, 2024 17:37

remove non releated changes

c8f262b

Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>

kaushalmahi12 mentioned this pull request May 16, 2024

[META] Multi-tenancy support in OpenSearch #13516

Open

9 tasks

sgup432 reviewed May 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add multitenant support for search workloads #13713

add multitenant support for search workloads #13713

kaushalmahi12 commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024

github-actions bot commented May 16, 2024

prudhvigodithi commented May 24, 2024

sgup432 May 22, 2024

sgup432 May 22, 2024

kaushalmahi12 May 28, 2024 •

edited

sgup432 May 28, 2024

sgup432 May 22, 2024

kaushalmahi12 May 28, 2024

sgup432 May 28, 2024

sgup432 May 28, 2024

kaushalmahi12 May 28, 2024

sgup432 May 28, 2024

kaushalmahi12 May 28, 2024

add multitenant support for search workloads #13713

Are you sure you want to change the base?

add multitenant support for search workloads #13713

Conversation

kaushalmahi12 commented May 16, 2024 • edited

Description

Related Issues

Check List

github-actions bot commented May 16, 2024

github-actions bot commented May 16, 2024

prudhvigodithi commented May 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaushalmahi12 May 28, 2024 • edited

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

Choose a reason for hiding this comment

kaushalmahi12 commented May 16, 2024 •

edited

kaushalmahi12 May 28, 2024 •

edited