You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/06/17 17:54:12 UTC

[GitHub] [pinot] klsince opened a new pull request, #8914: add api to check segment storage tier

klsince opened a new pull request, #8914:
URL: https://github.com/apache/pinot/pull/8914

   This PR adds two APIs to check immutable segment's current storage tier from servers. It's like how table size is calculated, reusing the multget util to get tier info from servers in parallel. 
   
   ## Release notes
   Two new APIs to check immutable segment's storage tier.
   1. /segments/{tableName}/{segmentName}/tier
   2. /segments/{tableName}/tier


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] klsince commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r903287363


##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableTierReader.java:
##########
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.util;
+
+import com.google.common.collect.BiMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executor;
+import javax.annotation.Nullable;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+
+
+/**
+ * Reads segment storage tiers from servers for the given table.
+ */
+public class TableTierReader {
+  private final Executor _executor;
+  private final HttpConnectionManager _connectionManager;
+  private final PinotHelixResourceManager _helixResourceManager;
+
+  public TableTierReader(Executor executor, HttpConnectionManager connectionManager,
+      PinotHelixResourceManager helixResourceManager) {
+    _executor = executor;
+    _connectionManager = connectionManager;
+    _helixResourceManager = helixResourceManager;
+  }
+
+  /**
+   * Get the segment storage tiers for the given table. The servers or segments not responding the request are
+   * recorded in the result to be checked by caller.
+   *
+   * @param tableNameWithType table name with type
+   * @param timeoutMs timeout for reading segment tiers from servers
+   * @return details of segment storage tiers for the given table
+   */
+  public TableTierDetails getTableTierDetails(String tableNameWithType, @Nullable String segmentName, int timeoutMs)
+      throws InvalidConfigException {
+    Map<String, List<String>> serverToSegmentsMap = new HashMap<>();
+    if (segmentName == null) {
+      serverToSegmentsMap.putAll(_helixResourceManager.getServerToSegmentsMap(tableNameWithType));
+    } else {
+      List<String> segmentInList = Collections.singletonList(segmentName);
+      for (String server : _helixResourceManager.getServers(tableNameWithType, segmentName)) {
+        serverToSegmentsMap.put(server, segmentInList);
+      }
+    }
+    BiMap<String, String> endpoints = _helixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
+    ServerTableTierReader serverTableTierReader = new ServerTableTierReader(_executor, _connectionManager);
+    Map<String, TableTierInfo> serverToTableTierInfoMap =
+        serverTableTierReader.getTableTierInfoFromServers(endpoints, tableNameWithType, timeoutMs);
+
+    TableTierDetails tableTierDetails = new TableTierDetails(tableNameWithType);
+    for (Map.Entry<String, List<String>> entry : serverToSegmentsMap.entrySet()) {
+      String server = entry.getKey();
+      TableTierInfo tableTierInfo = serverToTableTierInfoMap.get(server);
+      if (tableTierInfo == null) {
+        tableTierDetails._missingServers.add(server);
+        continue;
+      }
+      Map<String, String> segmentTiers = tableTierInfo.getSegmentTiers();
+      for (String expectedSegment : entry.getValue()) {
+        if (!segmentTiers.containsKey(expectedSegment)) {
+          tableTierDetails._missingSegments.computeIfAbsent(server, (k) -> new HashSet<>()).add(expectedSegment);
+        } else {
+          tableTierDetails._segmentTiers.computeIfAbsent(expectedSegment, (k) -> new HashMap<>())
+              .put(server, segmentTiers.get(expectedSegment));
+        }
+      }
+    }
+    return tableTierDetails;
+  }
+
+  // This class aggregates the TableTierInfo returned from multi servers.
+  public static class TableTierDetails {
+    private final String _tableName;
+    private final Set<String> _missingServers = new HashSet<>();

Review Comment:
   lemme know your thoughts?
   ```
   {
     "segmentTiers": {
       "seg01": {
         "server1": null,
         "server0": null
       },
       "seg02": {
         "server0": "someTier"
         "server2": "NO_RESPONSE_FROM_SERVER"
       },
       "seg03": {
         "server4": "SEGMENT_MISSED_ON_SERVER"
       }
     },
     "tableName": "myTable_OFFLINE"
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r904412107


##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableTierReader.java:
##########
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.util;
+
+import com.google.common.collect.BiMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executor;
+import javax.annotation.Nullable;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+
+
+/**
+ * Reads segment storage tiers from servers for the given table.
+ */
+public class TableTierReader {
+  private final Executor _executor;
+  private final HttpConnectionManager _connectionManager;
+  private final PinotHelixResourceManager _helixResourceManager;
+
+  public TableTierReader(Executor executor, HttpConnectionManager connectionManager,
+      PinotHelixResourceManager helixResourceManager) {
+    _executor = executor;
+    _connectionManager = connectionManager;
+    _helixResourceManager = helixResourceManager;
+  }
+
+  /**
+   * Get the segment storage tiers for the given table. The servers or segments not responding the request are
+   * recorded in the result to be checked by caller.
+   *
+   * @param tableNameWithType table name with type
+   * @param timeoutMs timeout for reading segment tiers from servers
+   * @return details of segment storage tiers for the given table
+   */
+  public TableTierDetails getTableTierDetails(String tableNameWithType, @Nullable String segmentName, int timeoutMs)
+      throws InvalidConfigException {
+    Map<String, List<String>> serverToSegmentsMap = new HashMap<>();
+    if (segmentName == null) {
+      serverToSegmentsMap.putAll(_helixResourceManager.getServerToSegmentsMap(tableNameWithType));
+    } else {
+      List<String> segmentInList = Collections.singletonList(segmentName);
+      for (String server : _helixResourceManager.getServers(tableNameWithType, segmentName)) {
+        serverToSegmentsMap.put(server, segmentInList);
+      }
+    }
+    BiMap<String, String> endpoints = _helixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
+    ServerTableTierReader serverTableTierReader = new ServerTableTierReader(_executor, _connectionManager);
+    Map<String, TableTierInfo> serverToTableTierInfoMap =
+        serverTableTierReader.getTableTierInfoFromServers(endpoints, tableNameWithType, timeoutMs);
+
+    TableTierDetails tableTierDetails = new TableTierDetails(tableNameWithType);
+    for (Map.Entry<String, List<String>> entry : serverToSegmentsMap.entrySet()) {
+      String server = entry.getKey();
+      TableTierInfo tableTierInfo = serverToTableTierInfoMap.get(server);
+      if (tableTierInfo == null) {
+        tableTierDetails._missingServers.add(server);
+        continue;
+      }
+      Map<String, String> segmentTiers = tableTierInfo.getSegmentTiers();
+      for (String expectedSegment : entry.getValue()) {
+        if (!segmentTiers.containsKey(expectedSegment)) {
+          tableTierDetails._missingSegments.computeIfAbsent(server, (k) -> new HashSet<>()).add(expectedSegment);
+        } else {
+          tableTierDetails._segmentTiers.computeIfAbsent(expectedSegment, (k) -> new HashMap<>())
+              .put(server, segmentTiers.get(expectedSegment));
+        }
+      }
+    }
+    return tableTierDetails;
+  }
+
+  // This class aggregates the TableTierInfo returned from multi servers.
+  public static class TableTierDetails {
+    private final String _tableName;
+    private final Set<String> _missingServers = new HashSet<>();

Review Comment:
   this looks great. easy to read it all in one place and get the whole picture, and would be easy to view alongside an ideal state πŸ‘ 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar merged pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
npawar merged PR #8914:
URL: https://github.com/apache/pinot/pull/8914


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] codecov-commenter commented on pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8914:
URL: https://github.com/apache/pinot/pull/8914#issuecomment-1159144574

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8914?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#8914](https://codecov.io/gh/apache/pinot/pull/8914?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (773e3b3) into [master](https://codecov.io/gh/apache/pinot/commit/080e849a6c68da8beb78ce9c9f943d7f5a8a8f78?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (080e849) will **decrease** coverage by `38.15%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #8914       +/-   ##
   =============================================
   - Coverage     63.05%   24.89%   -38.16%     
   + Complexity     4665       47     -4618     
   =============================================
     Files          1761     1801       +40     
     Lines         92215    94094     +1879     
     Branches      13825    14047      +222     
   =============================================
   - Hits          58142    23422    -34720     
   - Misses        29905    68395    +38490     
   + Partials       4168     2277     -1891     
   ```
   
   | Flag | Coverage Ξ” | |
   |---|---|---|
   | integration2 | `24.89% <0.00%> (?)` | |
   | unittests1 | `?` | |
   | unittests2 | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8914?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ” | |
   |---|---|---|
   | [.../pinot/common/restlet/resources/TableTierInfo.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvVGFibGVUaWVySW5mby5qYXZh) | `0.00% <0.00%> (ΓΈ)` | |
   | [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `19.34% <0.00%> (+6.84%)` | :arrow_up: |
   | [...e/pinot/controller/util/ServerTableTierReader.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlcnZlclRhYmxlVGllclJlYWRlci5qYXZh) | `0.00% <0.00%> (ΓΈ)` | |
   | [.../apache/pinot/controller/util/TableTierReader.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlVGllclJlYWRlci5qYXZh) | `0.00% <0.00%> (ΓΈ)` | |
   | [.../pinot/server/api/resources/TableTierResource.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZVRpZXJSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ΓΈ)` | |
   | [...in/java/org/apache/pinot/spi/utils/StringUtil.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvU3RyaW5nVXRpbC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1478 more](https://codecov.io/gh/apache/pinot/pull/8914/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8914?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Ξ” = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8914?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [080e849...773e3b3](https://codecov.io/gh/apache/pinot/pull/8914?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r901929458


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,53 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    LOGGER.info("Received a request to get storage tier for all segments for table {}", tableName);
+    return getTableTierInternal(tableName, null, tableTypeStr);
+  }
+
+  @GET
+  @Path("segments/{tableName}/{segmentName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tiers for the given segment", notes = "Get storage tiers for the given segment")
+  public TableTierReader.TableTierDetails getSegmentTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    segmentName = URIUtils.decode(segmentName);
+    LOGGER.info("Received a request to get storage tier for segment {} in table {}", segmentName, tableName);
+    return getTableTierInternal(tableName, segmentName, tableTypeStr);
+  }
+
+  private TableTierReader.TableTierDetails getTableTierInternal(String tableName, @Nullable String segmentName,
+      String tableTypeStr) {

Review Comment:
   tableTypeStr also Nullable?



##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/TableTierInfo.java:
##########
@@ -0,0 +1,46 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.Map;
+
+
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class TableTierInfo {
+  private final String _tableName;
+  private final Map<String, String> _segmentTiers;

Review Comment:
   please add some description in form of JsonPropertyDescription



##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,53 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    LOGGER.info("Received a request to get storage tier for all segments for table {}", tableName);
+    return getTableTierInternal(tableName, null, tableTypeStr);
+  }
+
+  @GET
+  @Path("segments/{tableName}/{segmentName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tiers for the given segment", notes = "Get storage tiers for the given segment")
+  public TableTierReader.TableTierDetails getSegmentTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    segmentName = URIUtils.decode(segmentName);
+    LOGGER.info("Received a request to get storage tier for segment {} in table {}", segmentName, tableName);
+    return getTableTierInternal(tableName, segmentName, tableTypeStr);
+  }
+
+  private TableTierReader.TableTierDetails getTableTierInternal(String tableName, @Nullable String segmentName,
+      String tableTypeStr) {
+    TableType tableType = Constants.validateTableType(tableTypeStr);
+    String tableNameWithType =

Review Comment:
   if user doesn't provide tableTypeStr, then we will only return results for OFFLINE table, even if it was a hybrid table. How about just making tableTypeStr a required prop in the APIs?



##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableTierReader.java:
##########
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.util;
+
+import com.google.common.collect.BiMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.Executor;
+import javax.annotation.Nullable;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+
+
+/**
+ * Reads segment storage tiers from servers for the given table.
+ */
+public class TableTierReader {
+  private final Executor _executor;
+  private final HttpConnectionManager _connectionManager;
+  private final PinotHelixResourceManager _helixResourceManager;
+
+  public TableTierReader(Executor executor, HttpConnectionManager connectionManager,
+      PinotHelixResourceManager helixResourceManager) {
+    _executor = executor;
+    _connectionManager = connectionManager;
+    _helixResourceManager = helixResourceManager;
+  }
+
+  /**
+   * Get the segment storage tiers for the given table. The servers or segments not responding the request are
+   * recorded in the result to be checked by caller.
+   *
+   * @param tableNameWithType table name with type
+   * @param timeoutMs timeout for reading segment tiers from servers
+   * @return details of segment storage tiers for the given table
+   */
+  public TableTierDetails getTableTierDetails(String tableNameWithType, @Nullable String segmentName, int timeoutMs)
+      throws InvalidConfigException {
+    Map<String, List<String>> serverToSegmentsMap = new HashMap<>();
+    if (segmentName == null) {
+      serverToSegmentsMap.putAll(_helixResourceManager.getServerToSegmentsMap(tableNameWithType));
+    } else {
+      List<String> segmentInList = Collections.singletonList(segmentName);
+      for (String server : _helixResourceManager.getServers(tableNameWithType, segmentName)) {
+        serverToSegmentsMap.put(server, segmentInList);
+      }
+    }
+    BiMap<String, String> endpoints = _helixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
+    ServerTableTierReader serverTableTierReader = new ServerTableTierReader(_executor, _connectionManager);
+    Map<String, TableTierInfo> serverToTableTierInfoMap =
+        serverTableTierReader.getTableTierInfoFromServers(endpoints, tableNameWithType, timeoutMs);
+
+    TableTierDetails tableTierDetails = new TableTierDetails(tableNameWithType);
+    for (Map.Entry<String, List<String>> entry : serverToSegmentsMap.entrySet()) {
+      String server = entry.getKey();
+      TableTierInfo tableTierInfo = serverToTableTierInfoMap.get(server);
+      if (tableTierInfo == null) {
+        tableTierDetails._missingServers.add(server);
+        continue;
+      }
+      Map<String, String> segmentTiers = tableTierInfo.getSegmentTiers();
+      for (String expectedSegment : entry.getValue()) {
+        if (!segmentTiers.containsKey(expectedSegment)) {
+          tableTierDetails._missingSegments.computeIfAbsent(server, (k) -> new HashSet<>()).add(expectedSegment);
+        } else {
+          tableTierDetails._segmentTiers.computeIfAbsent(expectedSegment, (k) -> new HashMap<>())
+              .put(server, segmentTiers.get(expectedSegment));
+        }
+      }
+    }
+    return tableTierDetails;
+  }
+
+  // This class aggregates the TableTierInfo returned from multi servers.
+  public static class TableTierDetails {
+    private final String _tableName;
+    private final Set<String> _missingServers = new HashSet<>();

Review Comment:
   splitting these details (missing servers, missing segments, responses from happy path) is nice when you want to look at just one of these. But I'm wondering, if this will make it too many places to look at to get the complete picture.
   wdyt about having all the info in _segmentTiers? the missingSegments and missingServers can be there in addition. so what i mean is, for the missing segment, put it in `segmentTiers` map, as segment->server,null/empty ? And also have an entry for any segment that was supposed to be found on the missingServer.
   If there's some gotcha in doing the above (like local tier is also represented as null?), then maybe just make `missingSegments` map as segment->server, so it is easier to read the 2 together?



##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TableTierResource.java:
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.server.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.restlet.resources.ResourceUtils;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.server.starter.ServerInstance;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * A server-side API to get the storage tiers of immutable segments of the given table from the server being requested.
+ */
+@Api(tags = "Table", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+    HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/")
+public class TableTierResource {
+
+  @Inject
+  private ServerInstance _serverInstance;
+
+  @GET
+  @Produces(MediaType.APPLICATION_JSON)
+  @Path("/tables/{tableName}/tier")

Review Comment:
   s/tableName/tableNameWithType ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] klsince commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r903012516


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,53 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    LOGGER.info("Received a request to get storage tier for all segments for table {}", tableName);
+    return getTableTierInternal(tableName, null, tableTypeStr);
+  }
+
+  @GET
+  @Path("segments/{tableName}/{segmentName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tiers for the given segment", notes = "Get storage tiers for the given segment")
+  public TableTierReader.TableTierDetails getSegmentTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    segmentName = URIUtils.decode(segmentName);
+    LOGGER.info("Received a request to get storage tier for segment {} in table {}", segmentName, tableName);
+    return getTableTierInternal(tableName, segmentName, tableTypeStr);
+  }
+
+  private TableTierReader.TableTierDetails getTableTierInternal(String tableName, @Nullable String segmentName,
+      String tableTypeStr) {
+    TableType tableType = Constants.validateTableType(tableTypeStr);
+    String tableNameWithType =

Review Comment:
   I kinda followed the convention of a few APIs like `/segments/.../crc` or `/segments/.../metadata`. But the convention is not very consistent, as some APIs process both tables if type is not set. I'd prefer to be consistent with crc/metadata APIs for the simplicity, but not feel very strong about it anyway.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r904410336


##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableTierReader.java:
##########
@@ -0,0 +1,115 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyDescription;
+import com.google.common.collect.BiMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import javax.annotation.Nullable;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+
+
+/**
+ * Reads segment storage tiers from servers for the given table.
+ */
+public class TableTierReader {
+  private static final String ERROR_RESP_NO_RESPONSE = "NO_RESPONSE_FROM_SERVER";
+  private static final String ERROR_RESP_MISSING_SEGMENT = "SEGMENT_MISSED_ON_SERVER";
+
+  private final Executor _executor;
+  private final HttpConnectionManager _connectionManager;
+  private final PinotHelixResourceManager _helixResourceManager;
+
+  public TableTierReader(Executor executor, HttpConnectionManager connectionManager,
+      PinotHelixResourceManager helixResourceManager) {
+    _executor = executor;
+    _connectionManager = connectionManager;
+    _helixResourceManager = helixResourceManager;
+  }
+
+  /**
+   * Get the segment storage tiers for the given table. The servers or segments not responding the request are
+   * recorded in the result to be checked by caller.
+   *
+   * @param tableNameWithType table name with type
+   * @param timeoutMs timeout for reading segment tiers from servers
+   * @return details of segment storage tiers for the given table
+   */
+  public TableTierDetails getTableTierDetails(String tableNameWithType, @Nullable String segmentName, int timeoutMs)
+      throws InvalidConfigException {
+    Map<String, List<String>> serverToSegmentsMap = new HashMap<>();
+    if (segmentName == null) {
+      serverToSegmentsMap.putAll(_helixResourceManager.getServerToSegmentsMap(tableNameWithType));
+    } else {
+      List<String> segmentInList = Collections.singletonList(segmentName);
+      for (String server : _helixResourceManager.getServers(tableNameWithType, segmentName)) {
+        serverToSegmentsMap.put(server, segmentInList);
+      }
+    }
+    BiMap<String, String> endpoints = _helixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
+    ServerTableTierReader serverTableTierReader = new ServerTableTierReader(_executor, _connectionManager);
+    Map<String, TableTierInfo> serverToTableTierInfoMap =

Review Comment:
   Looks like even we will always get info for every segment on the server, even if we had invoked the API with segmentName given. Should that lil bit of more optimization be done?  acquireAll/release each, although light, will be unnecessary processing we can avoid?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] klsince commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r904521472


##########
pinot-controller/src/main/java/org/apache/pinot/controller/util/TableTierReader.java:
##########
@@ -0,0 +1,115 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonPropertyDescription;
+import com.google.common.collect.BiMap;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import javax.annotation.Nullable;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+
+
+/**
+ * Reads segment storage tiers from servers for the given table.
+ */
+public class TableTierReader {
+  private static final String ERROR_RESP_NO_RESPONSE = "NO_RESPONSE_FROM_SERVER";
+  private static final String ERROR_RESP_MISSING_SEGMENT = "SEGMENT_MISSED_ON_SERVER";
+
+  private final Executor _executor;
+  private final HttpConnectionManager _connectionManager;
+  private final PinotHelixResourceManager _helixResourceManager;
+
+  public TableTierReader(Executor executor, HttpConnectionManager connectionManager,
+      PinotHelixResourceManager helixResourceManager) {
+    _executor = executor;
+    _connectionManager = connectionManager;
+    _helixResourceManager = helixResourceManager;
+  }
+
+  /**
+   * Get the segment storage tiers for the given table. The servers or segments not responding the request are
+   * recorded in the result to be checked by caller.
+   *
+   * @param tableNameWithType table name with type
+   * @param timeoutMs timeout for reading segment tiers from servers
+   * @return details of segment storage tiers for the given table
+   */
+  public TableTierDetails getTableTierDetails(String tableNameWithType, @Nullable String segmentName, int timeoutMs)
+      throws InvalidConfigException {
+    Map<String, List<String>> serverToSegmentsMap = new HashMap<>();
+    if (segmentName == null) {
+      serverToSegmentsMap.putAll(_helixResourceManager.getServerToSegmentsMap(tableNameWithType));
+    } else {
+      List<String> segmentInList = Collections.singletonList(segmentName);
+      for (String server : _helixResourceManager.getServers(tableNameWithType, segmentName)) {
+        serverToSegmentsMap.put(server, segmentInList);
+      }
+    }
+    BiMap<String, String> endpoints = _helixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
+    ServerTableTierReader serverTableTierReader = new ServerTableTierReader(_executor, _connectionManager);
+    Map<String, TableTierInfo> serverToTableTierInfoMap =

Review Comment:
   Good catch. Will pass segment name down to server side API to avoid unnecessary processing. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] klsince commented on pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
klsince commented on PR #8914:
URL: https://github.com/apache/pinot/pull/8914#issuecomment-1159110603

   Example response
   ```
   {
     "tableName": "baseballStats_OFFLINE",
     "missingSegments": {},
     "missingServers": [],
     "segmentTiers": {
       "baseballStats_OFFLINE_1604620800000_1604620800000_0": {
         "Server_192.168.1.58_7000": null /* null for default tier or sth like `mytier` if configured explicitly */
       }
     }
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] jackjlli commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
jackjlli commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r900500140


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,55 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "

Review Comment:
   `sed 's/tier/tiers/'`



##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TableTierResource.java:
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.server.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.restlet.resources.ResourceUtils;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.server.starter.ServerInstance;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * API to get the storage tiers of immutable segments of the given table.

Review Comment:
   It'd be good to mention here that this is a pinot server API.



##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,55 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(

Review Comment:
   Same here.



##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,55 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    LOGGER.info("Received a request to get storage tier for all segments for table {}", tableName);
+    return getTableTierInternal(tableName, null, tableTypeStr);
+  }
+
+  @GET
+  @Path("segments/{tableName}/{segmentName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tiers for the given segment", notes = "Get storage tiers for the given segment")
+  public TableTierReader.TableTierDetails getSegmentTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    segmentName = URIUtils.decode(segmentName);
+    LOGGER.info("Received a request to get storage tier for segment {} in table {}", segmentName, tableName);
+    return getTableTierInternal(tableName, segmentName, tableTypeStr);
+  }
+
+  private TableTierReader.TableTierDetails getTableTierInternal(String tableName, @Nullable String segmentName,
+      String tableTypeStr) {
+    TableType tableType = Constants.validateTableType(tableTypeStr);
+    String tableNameWithType =
+        ResourceUtils.getExistingTableNamesWithType(_pinotHelixResourceManager, tableName, tableType, LOGGER).get(0);
+    TableTierReader tableTierReader = new TableTierReader(_executor, _connectionManager, _pinotHelixResourceManager);
+    TableTierReader.TableTierDetails tableTierDetails;
+    try {
+      tableTierDetails = tableTierReader.getTableTierDetails(tableNameWithType, segmentName,
+          _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+    } catch (Throwable t) {
+      throw new ControllerApplicationException(LOGGER, String.format("Failed to get table tier for %s", tableName),

Review Comment:
   also print segment name here?



##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,55 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    LOGGER.info("Received a request to get storage tier for all segments for table {}", tableName);
+    return getTableTierInternal(tableName, null, tableTypeStr);
+  }
+
+  @GET
+  @Path("segments/{tableName}/{segmentName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tiers for the given segment", notes = "Get storage tiers for the given segment")
+  public TableTierReader.TableTierDetails getSegmentTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    segmentName = URIUtils.decode(segmentName);
+    LOGGER.info("Received a request to get storage tier for segment {} in table {}", segmentName, tableName);
+    return getTableTierInternal(tableName, segmentName, tableTypeStr);
+  }
+
+  private TableTierReader.TableTierDetails getTableTierInternal(String tableName, @Nullable String segmentName,
+      String tableTypeStr) {
+    TableType tableType = Constants.validateTableType(tableTypeStr);
+    String tableNameWithType =
+        ResourceUtils.getExistingTableNamesWithType(_pinotHelixResourceManager, tableName, tableType, LOGGER).get(0);
+    TableTierReader tableTierReader = new TableTierReader(_executor, _connectionManager, _pinotHelixResourceManager);
+    TableTierReader.TableTierDetails tableTierDetails;
+    try {
+      tableTierDetails = tableTierReader.getTableTierDetails(tableNameWithType, segmentName,
+          _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+    } catch (Throwable t) {
+      throw new ControllerApplicationException(LOGGER, String.format("Failed to get table tier for %s", tableName),
+          Response.Status.INTERNAL_SERVER_ERROR, t);
+    }
+    if (tableTierDetails == null) {

Review Comment:
   `tableTierDetails` doesn't seem to be null at all?



##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TableTierResource.java:
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.server.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.restlet.resources.ResourceUtils;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.server.starter.ServerInstance;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * API to get the storage tiers of immutable segments of the given table.
+ */
+@Api(tags = "Table", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+    HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/")
+public class TableTierResource {
+
+  @Inject
+  private ServerInstance _serverInstance;
+
+  @GET
+  @Produces(MediaType.APPLICATION_JSON)
+  @Path("/tables/{tableName}/tier")
+  @ApiOperation(value = "Get storage tiers of immutable segments of the given table", notes = "Get storage tiers of "
+      + "immutable segments of the given table")
+  @ApiResponses(value = {
+      @ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error"),
+      @ApiResponse(code = 404, message = "Table not found")
+  })
+  public String getTableTier(
+      @ApiParam(value = "Table Name with type", required = true) @PathParam("tableName") String tableName,

Review Comment:
   Why not add the table type as one of the params here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] klsince commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r903103150


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,53 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    LOGGER.info("Received a request to get storage tier for all segments for table {}", tableName);
+    return getTableTierInternal(tableName, null, tableTypeStr);
+  }
+
+  @GET
+  @Path("segments/{tableName}/{segmentName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tiers for the given segment", notes = "Get storage tiers for the given segment")
+  public TableTierReader.TableTierDetails getSegmentTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    segmentName = URIUtils.decode(segmentName);
+    LOGGER.info("Received a request to get storage tier for segment {} in table {}", segmentName, tableName);
+    return getTableTierInternal(tableName, segmentName, tableTypeStr);
+  }
+
+  private TableTierReader.TableTierDetails getTableTierInternal(String tableName, @Nullable String segmentName,
+      String tableTypeStr) {
+    TableType tableType = Constants.validateTableType(tableTypeStr);
+    String tableNameWithType =

Review Comment:
   good point. think I'll make tableType required to keep the API straightforward. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] klsince commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r900572607


##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TableTierResource.java:
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.server.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.restlet.resources.ResourceUtils;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.server.starter.ServerInstance;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * API to get the storage tiers of immutable segments of the given table.
+ */
+@Api(tags = "Table", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+    HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/")
+public class TableTierResource {
+
+  @Inject
+  private ServerInstance _serverInstance;
+
+  @GET
+  @Produces(MediaType.APPLICATION_JSON)
+  @Path("/tables/{tableName}/tier")
+  @ApiOperation(value = "Get storage tiers of immutable segments of the given table", notes = "Get storage tiers of "
+      + "immutable segments of the given table")
+  @ApiResponses(value = {
+      @ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error"),
+      @ApiResponse(code = 404, message = "Table not found")
+  })
+  public String getTableTier(
+      @ApiParam(value = "Table Name with type", required = true) @PathParam("tableName") String tableName,

Review Comment:
   This server-side API is supposed to be called by Controller, also following the convention of TableSizeResource.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] klsince commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
klsince commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r903000871


##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TableTierResource.java:
##########
@@ -0,0 +1,106 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.server.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.inject.Inject;
+import javax.ws.rs.DefaultValue;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.restlet.resources.ResourceUtils;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.server.starter.ServerInstance;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * A server-side API to get the storage tiers of immutable segments of the given table from the server being requested.
+ */
+@Api(tags = "Table", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+    HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/")
+public class TableTierResource {
+
+  @Inject
+  private ServerInstance _serverInstance;
+
+  @GET
+  @Produces(MediaType.APPLICATION_JSON)
+  @Path("/tables/{tableName}/tier")

Review Comment:
   yeah, thought about it, but decided to use `{tableName}` to be consistent with the other existing server side APIs..



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r903039409


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,53 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET
+  @Path("segments/{tableName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tier for all segments in the given table", notes = "Get storage tier for all "
+      + "segments in the given table")
+  public TableTierReader.TableTierDetails getTableTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    LOGGER.info("Received a request to get storage tier for all segments for table {}", tableName);
+    return getTableTierInternal(tableName, null, tableTypeStr);
+  }
+
+  @GET
+  @Path("segments/{tableName}/{segmentName}/tier")
+  @Produces(MediaType.APPLICATION_JSON)
+  @ApiOperation(value = "Get storage tiers for the given segment", notes = "Get storage tiers for the given segment")
+  public TableTierReader.TableTierDetails getSegmentTier(
+      @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+      @ApiParam(value = "Name of the segment", required = true) @PathParam("segmentName") @Encoded String segmentName,
+      @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+    segmentName = URIUtils.decode(segmentName);
+    LOGGER.info("Received a request to get storage tier for segment {} in table {}", segmentName, tableName);
+    return getTableTierInternal(tableName, segmentName, tableTypeStr);
+  }
+
+  private TableTierReader.TableTierDetails getTableTierInternal(String tableName, @Nullable String segmentName,
+      String tableTypeStr) {
+    TableType tableType = Constants.validateTableType(tableTypeStr);
+    String tableNameWithType =

Review Comment:
   i think my comment is less about which convention you go with, and more about we will skip half the data in the case where it's a hybrid table but no type is provided. we can keep this convention, but then return an array for both tables in the response?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar commented on a diff in pull request #8914: add api to check segment storage tier

Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #8914:
URL: https://github.com/apache/pinot/pull/8914#discussion_r905571376


##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TableTierResource.java:
##########
@@ -0,0 +1,153 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.server.api.resources;
+
+import io.swagger.annotations.Api;
+import io.swagger.annotations.ApiKeyAuthDefinition;
+import io.swagger.annotations.ApiOperation;
+import io.swagger.annotations.ApiParam;
+import io.swagger.annotations.ApiResponse;
+import io.swagger.annotations.ApiResponses;
+import io.swagger.annotations.Authorization;
+import io.swagger.annotations.SecurityDefinition;
+import io.swagger.annotations.SwaggerDefinition;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import javax.inject.Inject;
+import javax.ws.rs.Encoded;
+import javax.ws.rs.GET;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import org.apache.pinot.common.restlet.resources.ResourceUtils;
+import org.apache.pinot.common.restlet.resources.TableTierInfo;
+import org.apache.pinot.common.utils.URIUtils;
+import org.apache.pinot.core.data.manager.InstanceDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.SegmentDataManager;
+import org.apache.pinot.segment.local.data.manager.TableDataManager;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.server.starter.ServerInstance;
+
+import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
+
+
+/**
+ * A server-side API to get the storage tiers of immutable segments of the given table from the server being requested.
+ */
+@Api(tags = "Table", authorizations = {@Authorization(value = SWAGGER_AUTHORIZATION_KEY)})
+@SwaggerDefinition(securityDefinition = @SecurityDefinition(apiKeyAuthDefinitions = @ApiKeyAuthDefinition(name =
+    HttpHeaders.AUTHORIZATION, in = ApiKeyAuthDefinition.ApiKeyLocation.HEADER, key = SWAGGER_AUTHORIZATION_KEY)))
+@Path("/")
+public class TableTierResource {
+
+  @Inject
+  private ServerInstance _serverInstance;
+
+  @GET
+  @Produces(MediaType.APPLICATION_JSON)
+  @Path("/tables/{tableName}/tiers")
+  @ApiOperation(value = "Get storage tiers of immutable segments of the given table", notes = "Get storage tiers of "
+      + "immutable segments of the given table")
+  @ApiResponses(value = {
+      @ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error"),
+      @ApiResponse(code = 404, message = "Table not found")
+  })
+  public String getTableTiers(
+      @ApiParam(value = "Table name with type", required = true) @PathParam("tableName") String tableName)
+      throws WebApplicationException {
+    InstanceDataManager instanceDataManager = _serverInstance.getInstanceDataManager();
+    if (instanceDataManager == null) {
+      throw new WebApplicationException("Invalid server initialization", Response.Status.INTERNAL_SERVER_ERROR);
+    }
+    TableDataManager tableDataManager = instanceDataManager.getTableDataManager(tableName);
+    if (tableDataManager == null) {
+      throw new WebApplicationException("Table: " + tableName + " is not found", Response.Status.NOT_FOUND);
+    }
+    Set<String> mutableSegments = new HashSet<>();
+    Map<String, String> segmentTiers = new HashMap<>();
+    List<SegmentDataManager> segmentDataManagers = tableDataManager.acquireAllSegments();
+    try {
+      for (SegmentDataManager segmentDataManager : segmentDataManagers) {
+        if (segmentDataManager instanceof ImmutableSegmentDataManager) {
+          ImmutableSegment immutableSegment = (ImmutableSegment) segmentDataManager.getSegment();
+          segmentTiers.put(immutableSegment.getSegmentName(), immutableSegment.getTier());
+        } else {
+          mutableSegments.add(segmentDataManager.getSegmentName());
+        }
+      }
+    } finally {
+      for (SegmentDataManager segmentDataManager : segmentDataManagers) {
+        tableDataManager.releaseSegment(segmentDataManager);
+      }
+    }
+    TableTierInfo tableTierInfo = new TableTierInfo(tableDataManager.getTableName(), segmentTiers, mutableSegments);
+    return ResourceUtils.convertToJsonString(tableTierInfo);
+  }
+
+  @GET
+  @Produces(MediaType.APPLICATION_JSON)
+  @Path("/segments/{tableName}/{segmentName}/tiers")

Review Comment:
   nit: this looks like it is actually tableNameWithType (in the above API too)



##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java:
##########
@@ -717,6 +720,55 @@ public String getServerMetadata(
     return segmentsMetadata;
   }
 
+  @GET

Review Comment:
   nit: please add to the swagger doc here, the expected error codes  



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org