Tenant rebalance and status tracking APIs #11128

shounakmk219 · 2023-07-18T11:49:28Z

Tenant Rebalance API

This API addresses the task to rebalance all the tables for a given tenant. Usually when we tag new servers to a tenant it gets tedious to rebalance each table under that tenant to utilise the newly added servers. This operation becomes impossible to handle if the tenant has high number of tables. The proposed utility will allow users to rebalance a tenant and track its progress with minimal operational overhead.

More details are captured in this doc : Tenant Rebalance

Path : /tenants/{tenantName}/rebalance
Method : POST
Sample request body :

{
  "tenantName": "DefaultTenant",
  "degreeOfParallelism": 2,
  "parallelWhitelist": [
    "airlineStats_OFFLINE"
  ],
  "parallelBlacklist": [
    "airlineStats_REALTIME"
  ],
  "verboseResult": false,
  "dryRun": false,
  "downtime": false,
  "reassignInstances": false,
  "includeConsuming": false,
  "bootstrap": false,
  "minAvailableReplicas": 1,
  "bestEfforts": false,
  "updateTargetTier": false,
  "externalViewCheckIntervalInMs": 1000,
  "externalViewStabilizationTimeoutInMs": 3600000
}

Sample response :

{
  "jobId": "dfbbebb7-1f62-497d-82a7-ded6e0d855e1",
  "rebalanceTableResults": {
    "airlineStats_OFFLINE": {
      "jobId": "2d4dc2da-1071-42b5-a20c-ac38a6d53fc4",
      "status": "NO_OP",
      "description": "Table is already balanced"
    },
    "airlineStats_REALTIME": {
      "jobId": "9284f137-29c1-4c5a-a113-17b90a484403",
      "status": "NO_OP",
      "description": "Table is already balanced"
    }
  }
}

Tenant Rebalance Status tracking API

Path : /tenants/rebalanceStatus/{jobId}
Method : GET
Sample response body :

{
  "tenantRebalanceProgressStats": {
    "startTimeMs": 1689679866904,
    "timeToFinishInSeconds": 0,
    "completionStatusMsg": "Successfully rebalanced tenant DefaultTenant.",
    "tableStatusMap": {
      "airlineStats_OFFLINE": "Table is already balanced",
      "airlineStats_REALTIME": "Table is already balanced"
    },
    "totalTables": 2,
    "remainingTables": 0,
    "tableRebalanceJobIdMap": {
      "airlineStats_OFFLINE": "2d4dc2da-1071-42b5-a20c-ac38a6d53fc4",
      "airlineStats_REALTIME": "9284f137-29c1-4c5a-a113-17b90a484403"
    }
  },
  "timeElapsedSinceStartInSeconds": 0
}

codecov-commenter · 2023-07-18T12:26:25Z

Codecov Report

Merging #11128 (1553a8d) into master (044588f) will decrease coverage by 0.01%.
Report is 35 commits behind head on master.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #11128      +/-   ##
==========================================
- Coverage    0.11%    0.11%   -0.01%     
==========================================
  Files        2229     2239      +10     
  Lines      119951   120412     +461     
  Branches    18171    18215      +44     
==========================================
  Hits          137      137              
- Misses     119794   120255     +461     
  Partials       20       20

Flag	Coverage Δ
integration1temurin11	`0.00% <0.00%> (ø)`
integration1temurin17	`0.00% <0.00%> (ø)`
integration1temurin20	`0.00% <0.00%> (ø)`
integration2temurin11	`0.00% <0.00%> (?)`
integration2temurin17	`0.00% <0.00%> (ø)`
integration2temurin20	`0.00% <0.00%> (ø)`
unittests1temurin11	`0.00% <0.00%> (ø)`
unittests1temurin17	`0.00% <0.00%> (ø)`
unittests1temurin20	`0.00% <0.00%> (ø)`
unittests2temurin11	`0.11% <0.00%> (-0.01%)`	⬇️
unittests2temurin17	`0.11% <0.00%> (-0.01%)`	⬇️
unittests2temurin20	`0.11% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
...mmon/metadata/controllerjob/ControllerJobType.java	`0.00% <0.00%> (ø)`
...apache/pinot/controller/BaseControllerStarter.java	`0.00% <0.00%> (ø)`
...ller/api/resources/PinotTenantRestletResource.java	`0.00% <0.00%> (ø)`
...pi/resources/TenantRebalanceJobStatusResponse.java	`0.00% <0.00%> (ø)`
...troller/helix/core/rebalance/RebalanceContext.java	`0.00% <0.00%> (ø)`
...ntroller/helix/core/rebalance/RebalanceResult.java	`0.00% <ø> (ø)`
...core/rebalance/tenant/DefaultTenantRebalancer.java	`0.00% <0.00%> (ø)`
.../core/rebalance/tenant/TenantRebalanceContext.java	`0.00% <0.00%> (ø)`
...core/rebalance/tenant/TenantRebalanceObserver.java	`0.00% <0.00%> (ø)`
...rebalance/tenant/TenantRebalanceProgressStats.java	`0.00% <0.00%> (ø)`
... and 4 more

... and 47 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

saurabhd336 · 2023-07-20T10:07:33Z

...in/java/org/apache/pinot/controller/helix/core/rebalance/tenant/DefaultTenantRebalancer.java

+      try {
+        Configuration config = extractRebalanceConfig(context);
+        config.setProperty(RebalanceConfigConstants.DRY_RUN, true);
+        rebalanceResult.put(table, _pinotHelixResourceManager.rebalanceTable(table, config, false));


Why are we rebalancing twice in this function?

I am trying to keep the behaviour equivalent to the table rebalance endpoint. The first rebalance runs in DRY_RUN mode so the table is not actually rebalanced but we get the expected state which the user can expect once the async operations are completed. The next rebalance calls will do the actual work.

saurabhd336 · 2023-07-20T10:13:33Z

...in/java/org/apache/pinot/controller/helix/core/rebalance/tenant/DefaultTenantRebalancer.java

+                config.setProperty(RebalanceConfigConstants.JOB_ID, rebalanceResult.get(table).getJobId());
+                rebalanceTable(table, config, observer);
+              }
+            } catch (NoSuchElementException ignore) {


We should break out of the while loop explicitly and not by throwing an exception.

Will change it

saurabhd336 · 2023-07-20T10:18:18Z

...in/java/org/apache/pinot/controller/helix/core/rebalance/tenant/DefaultTenantRebalancer.java

+      _executorService.submit(() -> {
+        while (!parallelQueue.isEmpty()) {
+          try {
+            Thread.sleep(5000);


This will hog a thread from the shared executorService. We should yield thread for other tasks while waiting for some condition

Hey @saurabhd336 thanks for pointing this out, I have made changes to avoid the thread hogging. Now the last active thread from the parallel threads will pick up the job to rebalance the sequential tables.

saurabhd336 · 2023-07-20T10:20:26Z

also adding @swaminathanmanish for review

saurabhd336 · 2023-07-26T11:36:42Z

...in/java/org/apache/pinot/controller/helix/core/rebalance/tenant/DefaultTenantRebalancer.java

+          }
+          // Last parallel thread to finish the table rebalance job will pick up the
+          // sequential table rebalance execution
+          if (activeThreads.decrementAndGet() == 0) {


clever! 👍

saurabhd336 · 2023-07-26T11:49:01Z

...ller/src/main/java/org/apache/pinot/controller/api/resources/PinotTenantRestletResource.java

@@ -103,6 +112,9 @@ public class PinotTenantRestletResource {
  @Inject
  ControllerMetrics _controllerMetrics;

+  @Inject
+  ExecutorService _executorService;


I'm not sure if using the same shared executorService (which is also being used to handle multi get API call responses) for parallelly running multiple blocking operation like table rebalance is a good idea.. Although I do see same executorService being used to run rebalance for single table.
Thoughts?

Makes sense. Even I referred to the single table rebalance implementation but agree with your point that it will affect the shared executorService in cases where the tenant has lot of tables and user runs the rebalance with high parallelism.

snleee

LGTM otherwise

snleee · 2023-08-07T05:44:41Z

...troller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalanceContext.java

+public class RebalanceContext {
+  @JsonProperty("dryRun")
+  @ApiModelProperty(example = "false")
+  private Boolean _dryRun = false;


Default should be dryRun=true to make the protection? What's the current behavior for the rebalance API?

On the table rebalance API its false by default. I am keeping all the configs and their defaults inline with the table rebalance API so that user gets consistent behaviour for similar APIs. But I agree with you point as tenant rebalance operation will have a bigger impact radius. For now I am inclined to keep the consistency with table rebalance API but let me know if you think otherwise and we can make dryRun=true by default.

snleee · 2023-08-07T05:48:31Z

...troller/src/main/java/org/apache/pinot/controller/helix/core/rebalance/RebalanceContext.java

+  @JsonProperty("dryRun")
+  @ApiModelProperty(example = "false")
+  private Boolean _dryRun = false;
+  @JsonProperty("reassignInstances")


Are the default values for configs are the same as the existing API? If so, let's add TODO comment to see if we can make the config easier.

Ideally, these configuration should be intelligently be picked up as much as possible. For instance, reassignInstances should be triggered if we detect the need (i.e. there is extra server in the tenant that is not part of instance assignment or there is server in the instance assignment that is no longer be part of tenant).

Yes, I am referring the existing table rebalance API configs and their default values.
Great point on the auto feeding the configs wherever possible! I can take a follow up item to enhance the configs so as to keep the scope of the current PR limited. Will add a TODO.
These config enhancements should be reflected at table rebalance as well as tenant rebalance at the same time so that we maintain the consistency.

Jackie-Jiang

Please add Authorize (introduced in #11016) annotation to the new added APIs

saurabhd336 reviewed Jul 20, 2023

View reviewed changes

saurabhd336 reviewed Jul 26, 2023

View reviewed changes

shounakmk219 force-pushed the tenant-rebalance branch from e2782c9 to 35a4a51 Compare July 28, 2023 13:16

shounakmk219 closed this Jul 28, 2023

shounakmk219 reopened this Jul 28, 2023

Jackie-Jiang added rest-api feature labels Jul 31, 2023

snleee approved these changes Aug 7, 2023

View reviewed changes

shounakmk219 added 16 commits August 7, 2023 16:15

endpoint to rebalance tables of a tenant

637a343

move tenant rebalance logic to a separate TenantRebalancer service

5ff751a

implement extractRebalanceConfig

78954e8

function rename

c897efe

observability

488b405

status reporting

0f532d3

moved tenant rebalance related classes to a separate package

0057a32

ignore null values in response on RebalanceResult

4f94013

bugfix, initiate the _remainingTables field

1b95ae4

integration test for tenant rebalance

8bd93f3

break while loop gracefully

e91b20a

reuse the last active parallel thread to run the sequential jobs

e183a53

metadata npe handling

c308cb6

maintain separate executor service for tenant rebalance operations

8cf2dcf

prioritise dimension tables

c42780a

var name fix

b362946

shounakmk219 force-pushed the tenant-rebalance branch from d34a09c to b362946 Compare August 7, 2023 11:12

todo comment added

2237467

Jackie-Jiang reviewed Aug 10, 2023

View reviewed changes

Add Authorize annotations to APIs

1553a8d

saurabhd336 approved these changes Aug 11, 2023

View reviewed changes

snleee merged commit d6b1b4f into apache:master Aug 11, 2023
22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tenant rebalance and status tracking APIs #11128

Tenant rebalance and status tracking APIs #11128

shounakmk219 commented Jul 18, 2023

codecov-commenter commented Jul 18, 2023 •

edited

Loading

saurabhd336 Jul 20, 2023

shounakmk219 Jul 20, 2023

saurabhd336 Jul 20, 2023

shounakmk219 Jul 20, 2023

saurabhd336 Jul 20, 2023

shounakmk219 Jul 26, 2023

saurabhd336 commented Jul 20, 2023

saurabhd336 Jul 26, 2023

saurabhd336 Jul 26, 2023

shounakmk219 Jul 27, 2023 •

edited

Loading

snleee left a comment

snleee Aug 7, 2023

shounakmk219 Aug 7, 2023

snleee Aug 7, 2023

shounakmk219 Aug 7, 2023

Jackie-Jiang left a comment

Tenant rebalance and status tracking APIs #11128

Tenant rebalance and status tracking APIs #11128

Conversation

shounakmk219 commented Jul 18, 2023

Tenant Rebalance API

Tenant Rebalance Status tracking API

codecov-commenter commented Jul 18, 2023 • edited Loading

Codecov Report

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

saurabhd336 commented Jul 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shounakmk219 Jul 27, 2023 • edited Loading

Choose a reason for hiding this comment

snleee left a 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

Jackie-Jiang left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 18, 2023 •

edited

Loading

shounakmk219 Jul 27, 2023 •

edited

Loading