This is an automated email from the ASF dual-hosted git repository.
gortiz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new ef3ad433cb add the ability to override plugins using an integer
priority (#15766)
ef3ad433cb is described below
commit ef3ad433cb3346f20c82b3bdf653bf99cf1a45a2
Author: Gonzalo Ortiz Jaureguizar <gor...@users.noreply.github.com>
AuthorDate: Tue May 13 18:48:50 2025 +0200
add the ability to override plugins using an integer priority (#15766)
---
.../pinot/segment/spi/index/IndexPlugin.java | 15 +++
.../pinot/segment/spi/index/IndexService.java | 33 ++++-
.../pinot/segment/spi/index/IndexServiceTest.java | 133 +++++++++++++++++++++
3 files changed, 178 insertions(+), 3 deletions(-)
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexPlugin.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexPlugin.java
index 714cd2717a..c6d1ffb1da 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexPlugin.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexPlugin.java
@@ -31,5 +31,20 @@ package org.apache.pinot.segment.spi.index;
* these services is by using Google AutoService. BloomIndexPlugin can be used
as example.
*/
public interface IndexPlugin<T extends IndexType<?, ?, ?>> {
+ int DEFAULT_PRIORITY = 0;
T getIndexType();
+
+ /**
+ * Returns the priority of this plugin.
+ *
+ * Two plugins with the same id and different priorities will be loaded in
the order of their priority. Higher
+ * priority values are loaded first. This is useful when two plugins
implement the same index type, but one of them
+ * is a more optimized version of the other. In that case, the more
optimized version should have a higher priority.
+ *
+ * Default priority is 0, which means that custom plugins that want to
override default plugins need to use a
+ * positive priority.
+ */
+ default int getPriority() {
+ return DEFAULT_PRIORITY;
+ }
}
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexService.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexService.java
index 08368f02b6..23aff87c8c 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexService.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexService.java
@@ -22,6 +22,7 @@ package org.apache.pinot.segment.spi.index;
import com.google.common.collect.ImmutableMap;
import java.util.ArrayList;
import java.util.Collections;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
@@ -30,6 +31,8 @@ import java.util.Optional;
import java.util.ServiceLoader;
import java.util.Set;
import javax.annotation.concurrent.ThreadSafe;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
/**
@@ -53,18 +56,42 @@ import javax.annotation.concurrent.ThreadSafe;
*/
@ThreadSafe
public class IndexService {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(IndexService.class);
private static volatile IndexService _instance = fromServiceLoader();
private final List<IndexType<?, ?, ?>> _allIndexes;
private final Map<String, IndexType<?, ?, ?>> _allIndexesById;
- private IndexService(Set<IndexPlugin<?>> allPlugins) {
- ImmutableMap.Builder<String, IndexType<?, ?, ?>> builder =
ImmutableMap.builder();
+ public IndexService(Set<IndexPlugin<?>> allPlugins) {
+ HashMap<String, IndexPlugin<?>> pluginsById = new HashMap<>();
for (IndexPlugin<?> plugin : allPlugins) {
IndexType<?, ?, ?> indexType = plugin.getIndexType();
- builder.put(indexType.getId().toLowerCase(Locale.US), indexType);
+ pluginsById.merge(indexType.getId().toLowerCase(Locale.US), plugin,
(older, newer) -> {
+ if (older == newer) {
+ return older;
+ }
+ IndexPlugin<?> winner;
+ IndexPlugin<?> loser;
+ if (older.getPriority() >= newer.getPriority()) {
+ winner = older;
+ loser = newer;
+ } else {
+ winner = newer;
+ loser = older;
+ }
+ LOGGER.info("Two index plugins found for index type {}. "
+ + "Using {} with priority {} instead of {} with priority {}",
+ indexType.getId(), winner.getClass().getCanonicalName(),
winner.getPriority(),
+ loser.getClass().getCanonicalName(), loser.getPriority());
+ return winner;
+ });
+ }
+
+ ImmutableMap.Builder<String, IndexType<?, ?, ?>> builder =
ImmutableMap.builder();
+ for (Map.Entry<String, IndexPlugin<?>> entry : pluginsById.entrySet()) {
+ builder.put(entry.getKey(), entry.getValue().getIndexType());
}
_allIndexesById = builder.build();
// Sort index types so that servers can loop over and process them in a
more deterministic order.
diff --git
a/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/IndexServiceTest.java
b/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/IndexServiceTest.java
new file mode 100644
index 0000000000..94dfd3a910
--- /dev/null
+++
b/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/index/IndexServiceTest.java
@@ -0,0 +1,133 @@
+/**
+ * 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.segment.spi.index;
+
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeSet;
+import javax.annotation.Nullable;
+import org.apache.pinot.segment.spi.ColumnMetadata;
+import org.apache.pinot.segment.spi.creator.IndexCreationContext;
+import org.apache.pinot.segment.spi.store.SegmentDirectory;
+import org.apache.pinot.spi.config.table.IndexConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.Schema;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.*;
+
+
+public class IndexServiceTest {
+
+ @Test
+ public void testPriorities() {
+ TestIndexType indexType1 = new TestIndexType("test");
+ TestIndexType indexType2 = new TestIndexType("test");
+
+ IndexPlugin<TestIndexType> plugin1 = new IndexPlugin<TestIndexType>() {
+ @Override
+ public TestIndexType getIndexType() {
+ return indexType1;
+ }
+ @Override
+ public int getPriority() {
+ return 0;
+ }
+ };
+ IndexPlugin<TestIndexType> plugin2 = new IndexPlugin<TestIndexType>() {
+ @Override
+ public TestIndexType getIndexType() {
+ return indexType2;
+ }
+
+ @Override
+ public int getPriority() {
+ return 1;
+ }
+ };
+
+ TreeSet<IndexPlugin<?>> ascendingSet = new
TreeSet<>(Comparator.comparingInt(IndexPlugin::getPriority));
+ ascendingSet.add(plugin1);
+ ascendingSet.add(plugin2);
+ IndexService indexService1 = new IndexService(ascendingSet);
+ assertSame(indexService1.get("test"), indexType2);
+
+ // Verifies that the order in the test doesn't actually matter
+ TreeSet<IndexPlugin<?>> descendingSet = new TreeSet<>((p1, p2) ->
p2.getPriority() - p1.getPriority());
+ descendingSet.add(plugin1);
+ descendingSet.add(plugin2);
+ IndexService indexService2 = new IndexService(descendingSet);
+ assertSame(indexService2.get("test"), indexType2);
+ }
+
+ private static class TestIndexType implements IndexType<IndexConfig,
IndexReader, IndexCreator> {
+ private final String _id;
+
+ public TestIndexType(String id) {
+ _id = id;
+ }
+
+ @Override
+ public String getId() {
+ return _id;
+ }
+
+ @Override
+ public Class<IndexConfig> getIndexConfigClass() {
+ return IndexConfig.class;
+ }
+
+ @Override
+ public IndexConfig getDefaultConfig() {
+ return new IndexConfig(true);
+ }
+
+ @Override
+ public Map<String, IndexConfig> getConfig(TableConfig tableConfig, Schema
schema) {
+ return Map.of();
+ }
+
+ @Override
+ public IndexCreator createIndexCreator(IndexCreationContext context,
IndexConfig indexConfig)
+ throws Exception {
+ throw new UnsupportedOperationException("Indexes of type " +
getClass().getName() + " should not be created");
+ }
+
+ @Override
+ public IndexReaderFactory<IndexReader> getReaderFactory() {
+ throw new UnsupportedOperationException(IndexType.class.getName() + "
should not be read");
+ }
+
+ @Override
+ public List<String> getFileExtensions(@Nullable ColumnMetadata
columnMetadata) {
+ return List.of("test");
+ }
+
+ @Override
+ public IndexHandler createIndexHandler(SegmentDirectory segmentDirectory,
+ Map<String, FieldIndexConfigs> configsByCol, @Nullable Schema schema,
@Nullable TableConfig tableConfig) {
+ throw new UnsupportedOperationException(IndexType.class.getName() + "
should not be created");
+ }
+
+ @Override
+ public void convertToNewFormat(TableConfig tableConfig, Schema schema) {
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
