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/08/04 10:37:07 UTC

[GitHub] [pinot] richardstartin commented on a diff in pull request #9159: Add id to MetadataKey so that we can change the key order

richardstartin commented on code in PR #9159:
URL: https://github.com/apache/pinot/pull/9159#discussion_r937629134


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataTable.java:
##########
@@ -90,64 +90,72 @@ enum MetadataValueType {
     INT, LONG, STRING
   }
 
-  /* The MetadataKey is used in V3, where we present metadata as Map<MetadataKey, String>
+  /* The MetadataKey is used since V3, where we present metadata as Map<MetadataKey, String>
    * ATTENTION:
-   *  - Don't change existing keys.
-   *  - Don't remove existing keys.
-   *  - Always add new keys to the end.
+   *  - Don't change the id of existing keys.
+   *  - Always add new keys with higher ids.
    *  Otherwise, backward compatibility will be broken.
    */
   enum MetadataKey {
-    UNKNOWN("unknown", MetadataValueType.STRING),
-    TABLE("table", MetadataValueType.STRING), // NOTE: this key is only used in PrioritySchedulerTest
-    NUM_DOCS_SCANNED("numDocsScanned", MetadataValueType.LONG),
-    NUM_ENTRIES_SCANNED_IN_FILTER("numEntriesScannedInFilter", MetadataValueType.LONG),
-    NUM_ENTRIES_SCANNED_POST_FILTER("numEntriesScannedPostFilter", MetadataValueType.LONG),
-    NUM_SEGMENTS_QUERIED("numSegmentsQueried", MetadataValueType.INT),
-    NUM_SEGMENTS_PROCESSED("numSegmentsProcessed", MetadataValueType.INT),
-    NUM_SEGMENTS_MATCHED("numSegmentsMatched", MetadataValueType.INT),
-    NUM_CONSUMING_SEGMENTS_QUERIED("numConsumingSegmentsQueried", MetadataValueType.INT),
-    MIN_CONSUMING_FRESHNESS_TIME_MS("minConsumingFreshnessTimeMs", MetadataValueType.LONG),
-    TOTAL_DOCS("totalDocs", MetadataValueType.LONG),
-    NUM_GROUPS_LIMIT_REACHED("numGroupsLimitReached", MetadataValueType.STRING),
-    TIME_USED_MS("timeUsedMs", MetadataValueType.LONG),
-    TRACE_INFO("traceInfo", MetadataValueType.STRING),
-    REQUEST_ID("requestId", MetadataValueType.LONG),
-    NUM_RESIZES("numResizes", MetadataValueType.INT),
-    RESIZE_TIME_MS("resizeTimeMs", MetadataValueType.LONG),
-    THREAD_CPU_TIME_NS("threadCpuTimeNs", MetadataValueType.LONG),
-    SYSTEM_ACTIVITIES_CPU_TIME_NS("systemActivitiesCpuTimeNs", MetadataValueType.LONG),
-    RESPONSE_SER_CPU_TIME_NS("responseSerializationCpuTimeNs", MetadataValueType.LONG),
-    NUM_SEGMENTS_PRUNED_BY_SERVER("numSegmentsPrunedByServer", MetadataValueType.INT),
-    NUM_SEGMENTS_PRUNED_INVALID("numSegmentsPrunedByInvalid", MetadataValueType.INT),
-    NUM_SEGMENTS_PRUNED_BY_LIMIT("numSegmentsPrunedByLimit", MetadataValueType.INT),
-    NUM_SEGMENTS_PRUNED_BY_VALUE("numSegmentsPrunedByValue", MetadataValueType.INT),
-    EXPLAIN_PLAN_NUM_EMPTY_FILTER_SEGMENTS("explainPlanNumEmptyFilterSegments", MetadataValueType.INT),
-    EXPLAIN_PLAN_NUM_MATCH_ALL_FILTER_SEGMENTS("explainPlanNumMatchAllFilterSegments", MetadataValueType.INT),
-    NUM_CONSUMING_SEGMENTS_PROCESSED("numConsumingSegmentsProcessed", MetadataValueType.INT),
-    NUM_CONSUMING_SEGMENTS_MATCHED("numConsumingSegmentsMatched", MetadataValueType.INT);
-
-    private static final MetadataKey[] VALUES;
+    UNKNOWN(0, "unknown", MetadataValueType.STRING),
+    TABLE(1, "table", MetadataValueType.STRING), // NOTE: this key is only used in PrioritySchedulerTest
+    NUM_DOCS_SCANNED(2, "numDocsScanned", MetadataValueType.LONG),
+    NUM_ENTRIES_SCANNED_IN_FILTER(3, "numEntriesScannedInFilter", MetadataValueType.LONG),
+    NUM_ENTRIES_SCANNED_POST_FILTER(4, "numEntriesScannedPostFilter", MetadataValueType.LONG),
+    NUM_SEGMENTS_QUERIED(5, "numSegmentsQueried", MetadataValueType.INT),
+    NUM_SEGMENTS_PROCESSED(6, "numSegmentsProcessed", MetadataValueType.INT),
+    NUM_SEGMENTS_MATCHED(7, "numSegmentsMatched", MetadataValueType.INT),
+    NUM_CONSUMING_SEGMENTS_QUERIED(8, "numConsumingSegmentsQueried", MetadataValueType.INT),
+    NUM_CONSUMING_SEGMENTS_PROCESSED(26, "numConsumingSegmentsProcessed", MetadataValueType.INT),
+    NUM_CONSUMING_SEGMENTS_MATCHED(27, "numConsumingSegmentsMatched", MetadataValueType.INT),
+    MIN_CONSUMING_FRESHNESS_TIME_MS(9, "minConsumingFreshnessTimeMs", MetadataValueType.LONG),
+    TOTAL_DOCS(10, "totalDocs", MetadataValueType.LONG),
+    NUM_GROUPS_LIMIT_REACHED(11, "numGroupsLimitReached", MetadataValueType.STRING),
+    TIME_USED_MS(12, "timeUsedMs", MetadataValueType.LONG),
+    TRACE_INFO(13, "traceInfo", MetadataValueType.STRING),
+    REQUEST_ID(14, "requestId", MetadataValueType.LONG),
+    NUM_RESIZES(15, "numResizes", MetadataValueType.INT),
+    RESIZE_TIME_MS(16, "resizeTimeMs", MetadataValueType.LONG),
+    THREAD_CPU_TIME_NS(17, "threadCpuTimeNs", MetadataValueType.LONG),
+    SYSTEM_ACTIVITIES_CPU_TIME_NS(18, "systemActivitiesCpuTimeNs", MetadataValueType.LONG),
+    RESPONSE_SER_CPU_TIME_NS(19, "responseSerializationCpuTimeNs", MetadataValueType.LONG),
+    NUM_SEGMENTS_PRUNED_BY_SERVER(20, "numSegmentsPrunedByServer", MetadataValueType.INT),
+    NUM_SEGMENTS_PRUNED_INVALID(21, "numSegmentsPrunedByInvalid", MetadataValueType.INT),
+    NUM_SEGMENTS_PRUNED_BY_LIMIT(22, "numSegmentsPrunedByLimit", MetadataValueType.INT),
+    NUM_SEGMENTS_PRUNED_BY_VALUE(23, "numSegmentsPrunedByValue", MetadataValueType.INT),
+    EXPLAIN_PLAN_NUM_EMPTY_FILTER_SEGMENTS(24, "explainPlanNumEmptyFilterSegments", MetadataValueType.INT),
+    EXPLAIN_PLAN_NUM_MATCH_ALL_FILTER_SEGMENTS(25, "explainPlanNumMatchAllFilterSegments", MetadataValueType.INT);
+
+    private static final MetadataKey[] ID_TO_ENUM_KEY_MAP;
     private static final Map<String, MetadataKey> NAME_TO_ENUM_KEY_MAP = new HashMap<>();
+
+    private final int _id;
     private final String _name;
     private final MetadataValueType _valueType;
 
-    MetadataKey(String name, MetadataValueType valueType) {
+    MetadataKey(int id, String name, MetadataValueType valueType) {
+      _id = id;
       _name = name;
       _valueType = valueType;
     }
 
-    // getByOrdinal returns an enum key for a given ordinal or null if the key does not exist.
+    /**
+     * Returns the MetadataKey for the given id, or {@code null} if the id does not exist.
+     */
     @Nullable
-    public static MetadataKey getByOrdinal(int ordinal) {
-      return VALUES[Math.min(ordinal, VALUES.length - 1)];
+    public static MetadataKey getById(int id) {
+      return id < ID_TO_ENUM_KEY_MAP.length ? ID_TO_ENUM_KEY_MAP[id] : null;

Review Comment:
   This appears to be an unnecessary change as there is a null element at the end of the map to deal with unmapped ordinals.



-- 
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