apurtell commented on code in PR #2624:
URL: https://github.com/apache/phoenix/pull/2624#discussion_r4198436466


##########
phoenix-core-client/src/main/java/org/apache/phoenix/index/vector/VectorIndexScorecard.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * 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.phoenix.index.vector;
+
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.query.QueryServicesOptions;
+import org.apache.phoenix.schema.PTable;
+import org.apache.phoenix.util.EnvironmentEdgeManager;
+import org.apache.phoenix.util.MetaDataUtil;
+import org.apache.phoenix.util.ReadOnlyProps;
+import org.apache.phoenix.util.SchemaUtil;
+
+/**
+ * Vector index scorecard reconciliation and drift detection.
+ * <p>
+ * Manages posting list population and reassignment counters in {@code 
SYSTEM.VECTOR_CENTROID}.
+ * Periodic reconciliation synchronizes inline maintenance statistics with 
exact index row counts.
+ */
+public final class VectorIndexScorecard {
+
+  /** Maximum acceptable coefficient of variation for posting list sizes 
before flagging drift. */
+  static final double SIZE_CV_THRESHOLD = 
ClusterSkewMetrics.SEVERE_SKEW_CV_THRESHOLD;
+  /** Maximum acceptable fraction of empty posting lists before flagging 
drift. */
+  static final double EMPTY_CENTROID_FRACTION_THRESHOLD = 0.25;
+  /** Maximum acceptable reassignment rate per indexed vector between 
reconciliations. */
+  static final double REASSIGN_RATE_THRESHOLD = 0.20;
+
+  private VectorIndexScorecard() {
+  }
+
+  /** Encapsulates drift evaluation results and statistical indicators for a 
generation. */
+  public static final class Assessment {
+    private final boolean drifted;
+    private final String reason;
+    private final double skewRatio;
+    private final double sizeCv;
+    private final double emptyCentroidFraction;
+    private final double reassignmentRate;
+    private final long population;
+
+    Assessment(boolean drifted, String reason, double skewRatio, double sizeCv,
+      double emptyCentroidFraction, double reassignmentRate, long population) {
+      this.drifted = drifted;
+      this.reason = reason;
+      this.skewRatio = skewRatio;
+      this.sizeCv = sizeCv;
+      this.emptyCentroidFraction = emptyCentroidFraction;
+      this.reassignmentRate = reassignmentRate;
+      this.population = population;
+    }
+
+    /** Returns true if any drift threshold was exceeded. */
+    public boolean isDrifted() {
+      return drifted;
+    }
+
+    /** Returns formatted summary of exceeded drift thresholds, or null if 
within limits. */
+    public String getReason() {
+      return reason;
+    }
+
+    /** Ratio of maximum posting list size to median list size. */
+    public double getSkewRatio() {
+      return skewRatio;
+    }
+
+    public double getSizeCv() {
+      return sizeCv;
+    }
+
+    public double getEmptyCentroidFraction() {
+      return emptyCentroidFraction;
+    }
+
+    public double getReassignmentRate() {
+      return reassignmentRate;
+    }
+
+    public long getPopulation() {
+      return population;
+    }
+
+    @Override
+    public String toString() {
+      return "Assessment{drifted=" + drifted + ", reason=" + reason + ", 
skewRatio=" + skewRatio
+        + ", sizeCv=" + sizeCv + ", emptyCentroidFraction=" + 
emptyCentroidFraction
+        + ", reassignmentRate=" + reassignmentRate + ", population=" + 
population + "}";
+    }
+  }
+
+  /**
+   * Reconciles scorecard statistics against physical index table row counts. 
Resets inline
+   * reassignment counters and updates last reconciliation timestamp.
+   * @param conn connection without tenant scoping to aggregate across all 
tenants
+   * @return reconciled scorecard rows with pre-reset reassignment metrics
+   */
+  public static List<ScorecardRow> reconcile(PhoenixConnection conn, PTable 
index, long generation)
+    throws SQLException {
+    String indexName = index.getName().getString();
+    String centroidColumn = "\"" + MetaDataUtil.VECTOR_CENTROID_ID_COLUMN_NAME 
+ "\"";
+    Map<Integer, Long> populations = new HashMap<>();
+    try (Statement stmt = conn.createStatement();
+      ResultSet rs = stmt.executeQuery("SELECT " + centroidColumn + ", 
COUNT(*) FROM "
+        + SchemaUtil.getEscapedFullTableName(indexName) + " GROUP BY " + 
centroidColumn)) {
+      while (rs.next()) {
+        populations.put(rs.getInt(1), rs.getLong(2));
+      }
+    }
+    List<ScorecardRow> before = CentroidManager.loadScorecard(conn, indexName, 
generation);
+    List<ScorecardRow> reconciled = new ArrayList<>(before.size());
+    List<ScorecardRow> reset = new ArrayList<>(before.size());
+    for (ScorecardRow row : before) {
+      long population = populations.getOrDefault(row.getCentroidId(), 0L);
+      reconciled.add(new ScorecardRow(row.getCentroidId(), population, 
row.getReassignCount()));
+      reset.add(new ScorecardRow(row.getCentroidId(), population, 0L));
+    }
+    CentroidManager.persistScorecard(conn, indexName, generation, reset);

Review Comment:
   Acknowledged. However, the fix just requires applying a relative correction, 
and we can use the atomic increment that flushes already employ, so no epoch 
protocol is required.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to