This is an automated email from the ASF dual-hosted git repository.
xiangfu0 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 63cd8e006d1 Let segment directories serve mapped segment metadata
(#20114)
63cd8e006d1 is described below
commit 63cd8e006d1e194ad072fc8a5fb1d7eae53d761d
Author: Xiang Fu <[email protected]>
AuthorDate: Mon Nov 2 13:27:10 2026 -0800
Let segment directories serve mapped segment metadata (#20114)
Mapped segment metadata (#19998) is only reachable for
SegmentLocalFSDirectory:
ImmutableSegmentLoader switches to it through an instanceof check, and the
mapped
SegmentMetadataImpl constructor is private behind loadMapped(), which always
returns the base class. Another SegmentDirectory implementation, or one
whose
metadata subclasses SegmentMetadataImpl, has no way to opt in.
- Add SegmentDirectory#loadMappedMetadata(), a no-op by default.
ImmutableSegmentLoader calls it, when mapped metadata is enabled, on any
directory after preprocessing and before the segment serves, so
preprocessing
keeps running on eager metadata for every implementation.
SegmentLocalFSDirectory's existing method now overrides it.
- Make the mapped SegmentMetadataImpl constructor protected. It behaves like
loadMapped(): the caller owns and closes the mapping.
- detachedCopy() keeps what getIndexDir() reports, so a subclass that hides
its
staging directory keeps hiding it in the copy.
---
.../immutable/ImmutableSegmentLoader.java | 6 +--
.../segment/store/SegmentLocalFSDirectory.java | 1 +
.../segment/index/SegmentMetadataImplTest.java | 60 ++++++++++++++++++++++
.../spi/index/metadata/SegmentMetadataImpl.java | 10 +++-
.../pinot/segment/spi/store/SegmentDirectory.java | 7 +++
5 files changed, 78 insertions(+), 6 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
index 40c4d47ea20..742d823e693 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/immutable/ImmutableSegmentLoader.java
@@ -33,7 +33,6 @@ import
org.apache.pinot.segment.local.segment.index.converter.SegmentFormatConve
import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig;
import org.apache.pinot.segment.local.segment.index.loader.SegmentPreProcessor;
import
org.apache.pinot.segment.local.segment.index.readers.text.MultiColumnLuceneTextIndexReader;
-import org.apache.pinot.segment.local.segment.store.SegmentLocalFSDirectory;
import
org.apache.pinot.segment.local.segment.virtualcolumn.VirtualColumnContext;
import
org.apache.pinot.segment.local.segment.virtualcolumn.VirtualColumnProvider;
import
org.apache.pinot.segment.local.segment.virtualcolumn.VirtualColumnProviderFactory;
@@ -213,9 +212,8 @@ public class ImmutableSegmentLoader {
@Nullable Schema schema)
throws Exception {
// Preprocessing uses mutable metadata. Only the final immutable serving
path uses the mapped representation.
- if (indexLoadingConfig.isMappedSegmentMetadataEnabled()
- && segmentDirectory instanceof SegmentLocalFSDirectory localDirectory)
{
- localDirectory.loadMappedMetadata();
+ if (indexLoadingConfig.isMappedSegmentMetadataEnabled()) {
+ segmentDirectory.loadMappedMetadata();
}
SegmentMetadataImpl segmentMetadata =
segmentDirectory.getSegmentMetadata();
if (segmentMetadata.getTotalDocs() == 0) {
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
index 864027eccc8..fd5ba12f2bd 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/SegmentLocalFSDirectory.java
@@ -166,6 +166,7 @@ public class SegmentLocalFSDirectory extends
SegmentDirectory {
}
/// Replaces mutable metadata with the mapped representation after
preprocessing and before the segment is queryable.
+ @Override
public void loadMappedMetadata()
throws IOException {
if (_segmentMetadata.isMapped()) {
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java
index e8a3362b927..34a7c16983e 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/SegmentMetadataImplTest.java
@@ -65,6 +65,7 @@ import
org.apache.pinot.segment.spi.index.metadata.SegmentMetadataImpl;
import org.apache.pinot.segment.spi.memory.PinotBufferFactory;
import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
import org.apache.pinot.segment.spi.store.ColumnIndexUtils;
+import org.apache.pinot.segment.spi.store.SegmentDirectory;
import org.apache.pinot.segment.spi.store.SegmentDirectoryPaths;
import org.apache.pinot.spi.config.instance.InstanceDataManagerConfig;
import org.apache.pinot.spi.config.table.FieldConfig;
@@ -89,6 +90,8 @@ import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertFalse;
@@ -495,6 +498,50 @@ public class SegmentMetadataImplTest {
assertEquals(PinotDataBuffer.getMmapBufferCount(), mappings);
}
+ @Test
+ public void testMappedMetadataSubclass()
+ throws Exception {
+ new SegmentV1V2ToV3FormatConverter().convert(_segmentDirectory);
+ SegmentMetadataImpl eager = new SegmentMetadataImpl(_segmentDirectory);
+ long mappings = PinotDataBuffer.getMmapBufferCount();
+ try (MetadataOnlySegmentMetadata mapped = new
MetadataOnlySegmentMetadata(_segmentDirectory)) {
+ assertTrue(mapped.isMapped());
+ assertNull(mapped.getIndexDir());
+ assertEquals(mapped.getName(), eager.getName());
+ assertEquals(mapped.getTotalDocs(), eager.getTotalDocs());
+ assertEquals(mapped.getPhysicalColumnNames(),
eager.getSchema().getPhysicalColumnNames());
+ for (String column : mapped.getPhysicalColumnNames()) {
+ ColumnMetadata actual = mapped.getColumnMetadataFor(column);
+ assertTrue(actual instanceof MappedColumnMetadata, column);
+ assertEquals(((MappedColumnMetadata) actual).getIndexSizeMap(),
+ ((ColumnMetadataImpl)
eager.getColumnMetadataFor(column)).getIndexSizeMap(), column);
+ }
+ SegmentMetadataImpl detached = mapped.detachedCopy();
+ assertFalse(detached.isMapped());
+ assertFalse(detached instanceof MetadataOnlySegmentMetadata);
+ assertNull(detached.getIndexDir());
+ assertEquals(detached.getTotalDocs(), eager.getTotalDocs());
+ }
+ assertEquals(PinotDataBuffer.getMmapBufferCount(), mappings);
+ }
+
+ @Test
+ public void testServingLoadSwitchesAnySegmentDirectoryToMappedMetadata()
+ throws Exception {
+ InstanceDataManagerConfig instanceConfig =
mock(InstanceDataManagerConfig.class);
+ PinotConfiguration config = new PinotConfiguration();
+ when(instanceConfig.getConfig()).thenReturn(config);
+ for (boolean enabled : new boolean[]{true, false}) {
+ config.setProperty(IndexLoadingConfig.MAPPED_SEGMENT_METADATA_ENABLED,
enabled);
+ // Not a SegmentLocalFSDirectory: the switch is part of the
SegmentDirectory contract.
+ SegmentDirectory segmentDirectory = mock(SegmentDirectory.class);
+ when(segmentDirectory.getSegmentMetadata()).thenReturn(
+ new SegmentMetadataImpl("testTable", "emptySegment", new Schema(),
System.currentTimeMillis()));
+ ImmutableSegmentLoader.load(segmentDirectory, new
IndexLoadingConfig(instanceConfig, null, null)).destroy();
+ verify(segmentDirectory, times(enabled ? 1 : 0)).loadMappedMetadata();
+ }
+ }
+
@Test
public void testMappedMetadataIgnoresStaleEagerSnapshot()
throws Exception {
@@ -881,4 +928,17 @@ public class SegmentMetadataImplTest {
driver.build();
return driver.getOutputDirectory();
}
+
+ /// Reports no index directory, as metadata staged apart from a segment's
index files would.
+ private static final class MetadataOnlySegmentMetadata extends
SegmentMetadataImpl {
+ MetadataOnlySegmentMetadata(File metadataDir)
+ throws IOException, ConfigurationException {
+ super(metadataDir, ReadMode.mmap, null);
+ }
+
+ @Override
+ public File getIndexDir() {
+ return null;
+ }
+ }
}
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java
index f781aebacd4..46391140d21 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java
@@ -207,7 +207,8 @@ public class SegmentMetadataImpl implements
SegmentMetadata, Closeable {
}
private SegmentMetadataImpl(SegmentMetadataImpl mapped) {
- _indexDir = mapped._indexDir;
+ // A subclass may report a different index directory than the one its
mapping reads.
+ _indexDir = mapped.getIndexDir();
_columnMetadataMap = new TreeMap<>();
_schema = new Schema();
_segmentName = mapped._segmentName;
@@ -263,7 +264,12 @@ public class SegmentMetadataImpl implements
SegmentMetadata, Closeable {
return MappedSegmentMetadataStore.isTempFileName(fileName);
}
- private SegmentMetadataImpl(File indexDir, ReadMode readMode, @Nullable
EagerSnapshot eagerSnapshot)
+ /// Mapped constructor for segment directory implementations that subclass
[SegmentMetadataImpl] to carry their own
+ /// metadata, for example one that stages a segment's metadata files locally
while its index files live elsewhere.
+ /// It reads `indexDir` exactly as [#loadMapped(File, ReadMode,
EagerSnapshot)] does, so the caller owns the mapping
+ /// and must [#close()] it. A subclass may override [#getIndexDir()];
[#detachedCopy()] keeps what it reports, but
+ /// returns a plain [SegmentMetadataImpl].
+ protected SegmentMetadataImpl(File indexDir, ReadMode readMode, @Nullable
EagerSnapshot eagerSnapshot)
throws IOException, ConfigurationException {
_indexDir = indexDir;
_columnMetadataMap = null;
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/store/SegmentDirectory.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/store/SegmentDirectory.java
index 2cda5a5bea3..277699cc873 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/store/SegmentDirectory.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/store/SegmentDirectory.java
@@ -101,6 +101,13 @@ public abstract class SegmentDirectory implements
Closeable {
public abstract void reloadMetadata()
throws Exception;
+ /// Switches to mapped segment metadata, for a directory that supports it,
after preprocessing and before the segment
+ /// is queryable. The serving load calls this only when mapped segment
metadata is enabled. The default keeps the
+ /// current metadata.
+ public void loadMappedMetadata()
+ throws IOException {
+ }
+
/// Get the path/URL for the directory
public abstract Path getPath();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
