Skip to content

Commit 8312455

Browse files
author
Vishwas Garg
committed
Foyer address PR comments
1 parent 3fcf13d commit 8312455

14 files changed

Lines changed: 310 additions & 1069 deletions

File tree

libs/vectorized-exec-spi/src/main/java/org/opensearch/vectorized/execution/jni/PageCacheAware.java

Lines changed: 0 additions & 32 deletions
This file was deleted.

libs/vectorized-exec-spi/src/main/java/org/opensearch/vectorized/execution/jni/PageCacheProvider.java

Lines changed: 0 additions & 53 deletions
This file was deleted.

modules/tiered-storage/src/main/java/org/opensearch/storage/TieredStoragePlugin.java

Lines changed: 2 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@
6666
import org.opensearch.telemetry.metrics.MetricsRegistry;
6767
import org.opensearch.telemetry.tracing.Tracer;
6868
import org.opensearch.index.store.CompositeStoreDirectoryFactory;
69-
import org.opensearch.vectorized.execution.jni.PageCacheAware;
70-
import org.opensearch.vectorized.execution.jni.PageCacheProvider;
7169
import org.opensearch.vectorized.execution.jni.NativeObjectStoreProvider;
7270

7371
import java.util.ArrayList;
@@ -90,7 +88,7 @@
9088
* Per-repository remote stores are added to the shared {@code FileRegistry} as new
9189
* repositories are encountered. Different indices can point to different repositories.
9290
*/
93-
public class TieredStoragePlugin extends Plugin implements IndexStorePlugin, ActionPlugin, TelemetryAwarePlugin, NativeObjectStoreProvider, PageCacheAware {
91+
public class TieredStoragePlugin extends Plugin implements IndexStorePlugin, ActionPlugin, TelemetryAwarePlugin, NativeObjectStoreProvider {
9492

9593
private static final org.apache.logging.log4j.Logger logger = org.apache.logging.log4j.LogManager.getLogger(TieredStoragePlugin.class);
9694

@@ -103,14 +101,6 @@ public class TieredStoragePlugin extends Plugin implements IndexStorePlugin, Act
103101

104102
private volatile Supplier<RepositoriesService> repositoriesServiceSupplier;
105103

106-
/**
107-
* Page cache provider — received from DataFusionPlugin via Node.java.
108-
* Passed into TieredCompositeStoreDirectoryFactory so that CachedParquetCacheStrategy
109-
* can be used for parquet format files instead of PassthroughCacheStrategy.
110-
* May be null if DataFusionPlugin is not loaded or page cache is disabled.
111-
*/
112-
private volatile PageCacheProvider pageCacheProvider;
113-
114104
// Global native ObjectStore — created lazily on first warm shard creation
115105
private volatile long globalObjStoreDataPtr;
116106
private volatile long globalObjStoreVtablePtr;
@@ -149,17 +139,6 @@ public Map<String, CompositeStoreDirectoryFactory> getCompositeStoreDirectoryFac
149139
return Collections.emptyMap();
150140
}
151141

152-
/**
153-
* Set the page cache provider (e.g. from DataFusionPlugin).
154-
* Called by Node.java after discovering a plugin implementing {@link PageCacheProvider},
155-
* so TieredCompositeStoreDirectoryFactory can use CachedParquetCacheStrategy.
156-
*/
157-
@Override
158-
public void setPageCacheProvider(PageCacheProvider provider) {
159-
this.pageCacheProvider = provider;
160-
logger.info("[TieredStoragePlugin] PageCacheProvider set — parquet reads will use page cache");
161-
}
162-
163142
@Override
164143
public Map<String, org.opensearch.index.store.CachedCompositeStoreDirectoryFactory> getCachedCompositeStoreDirectoryFactories() {
165144
return Map.of(TIERED_COMPOSITE_INDEX_TYPE, new TieredCompositeStoreDirectoryFactory(
@@ -168,12 +147,7 @@ public Map<String, org.opensearch.index.store.CachedCompositeStoreDirectoryFacto
168147
ensureGlobalObjectStoreCreated();
169148
ensureRemoteStoreForRepo(repoName);
170149
return globalRegistryPtr;
171-
},
172-
// Pass as Supplier so it is resolved LAZILY at shard creation time (newDirectory()).
173-
// getCachedCompositeStoreDirectoryFactories() is called in Node.java at line ~973
174-
// BEFORE setPageCacheProvider() is called at line ~1191.
175-
// The Supplier captures 'this' and reads the volatile field at call time, not now.
176-
() -> this.pageCacheProvider
150+
}
177151
));
178152
}
179153

0 commit comments

Comments
 (0)