This is an automated email from the ASF dual-hosted git repository.
voonhous pushed a commit to branch release-1.2.1
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/release-1.2.1 by this push:
new 7b411aa2b919 fix(trino): unbind HudiCacheKeyProvider on 1.2.1 (#19985)
7b411aa2b919 is described below
commit 7b411aa2b919da66fdeb3ebf92b8e037bfba8873
Author: voonhous <[email protected]>
AuthorDate: Thu Sep 17 20:50:31 2026 +0800
fix(trino): unbind HudiCacheKeyProvider on 1.2.1 (#19985)
HudiCacheKeyProvider implements the Trino 483 CacheKeyProvider
contract. After 483, getCacheKey returns Optional<CacheKey> instead of
Optional<String>. The erased method descriptor is identical, so a
connector built at 483 still links in a plugin assembled against a
newer Trino, then fails with ClassCastException at the first cached
read. Leave the provider unbound so Trino's default provider (path,
last-modified time and length) is used; caching stays on.
The testSelectWithFilter and testJoin op-count assertions in
TestHudiAlluxioCacheFileOperations and TestHudiMemoryCacheFileOperations
encode the provider's exclusions (table properties and index
definitions re-read on every query), so they are disabled with the
reason in the annotation; they return with the binding. Add
testReadsServedFromMemoryCache so the metadata memory cache keeps
count-independent coverage alongside testReadsServedFromAlluxioCache.
This is release-only: master is on the new contract and keeps the
binding.
---
.../main/java/io/trino/plugin/hudi/HudiModule.java | 8 +++---
.../hudi/TestHudiAlluxioCacheFileOperations.java | 3 +++
.../hudi/TestHudiMemoryCacheFileOperations.java | 30 ++++++++++++++++++++++
3 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java
b/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java
index cd17429b90d3..7dec679db63d 100644
--- a/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java
+++ b/hudi-trino/src/main/java/io/trino/plugin/hudi/HudiModule.java
@@ -19,7 +19,6 @@ import com.google.inject.Module;
import com.google.inject.Provides;
import com.google.inject.Scopes;
import com.google.inject.Singleton;
-import io.trino.filesystem.cache.CacheKeyProvider;
import io.trino.metastore.HiveMetastore;
import io.trino.plugin.base.metrics.FileFormatDataSourceStats;
import io.trino.plugin.base.session.SessionPropertiesProvider;
@@ -28,7 +27,6 @@ import io.trino.plugin.hive.HiveNodePartitioningProvider;
import io.trino.plugin.hive.HiveTransactionHandle;
import io.trino.plugin.hive.parquet.ParquetReaderConfig;
import io.trino.plugin.hive.parquet.ParquetWriterConfig;
-import io.trino.plugin.hudi.cache.HudiCacheKeyProvider;
import io.trino.plugin.hudi.stats.ForHudiTableStatistics;
import io.trino.spi.connector.ConnectorNodePartitioningProvider;
import io.trino.spi.connector.ConnectorPageSourceProvider;
@@ -40,7 +38,6 @@ import java.util.concurrent.ScheduledExecutorService;
import java.util.function.BiFunction;
import static com.google.inject.multibindings.Multibinder.newSetBinder;
-import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder;
import static io.airlift.concurrent.Threads.daemonThreadsNamed;
import static io.airlift.configuration.ConfigBinder.configBinder;
import static io.airlift.bootstrap.ClosingBinder.closingBinder;
@@ -75,7 +72,10 @@ public class HudiModule
binder.bind(FileFormatDataSourceStats.class).in(Scopes.SINGLETON);
newExporter(binder).export(FileFormatDataSourceStats.class).withGeneratedName();
- newOptionalBinder(binder,
CacheKeyProvider.class).setBinding().to(HudiCacheKeyProvider.class).in(Scopes.SINGLETON);
+ // HudiCacheKeyProvider is deliberately not bound on release-1.2.1, so
Trino's default
+ // provider is used. It implements the Trino 483 CacheKeyProvider
contract, which changed
+ // after 483; binding it breaks a plugin assembled against a newer
Trino at the first cached
+ // read. Restore the binding when the connector targets a Trino with
the new contract.
closingBinder(binder).registerExecutor(Key.get(ExecutorService.class,
ForHudiTableStatistics.class));
closingBinder(binder).registerExecutor(Key.get(ExecutorService.class,
ForHudiSplitManager.class));
diff --git
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
index 22f569dcf50d..03e579f7bbce 100644
---
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
+++
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiAlluxioCacheFileOperations.java
@@ -24,6 +24,7 @@ import io.trino.testing.AbstractTestQueryFramework;
import io.trino.testing.DistributedQueryRunner;
import io.trino.testing.QueryRunner;
import org.intellij.lang.annotations.Language;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
@@ -82,6 +83,7 @@ public class TestHudiAlluxioCacheFileOperations
}
@Test
+ @Disabled("Asserts the exclusions of HudiCacheKeyProvider (table
properties and index definitions are re-read from storage on every query).
release-1.2.1 does not bind that provider, so Trino's default provider caches
those files and the counts no longer hold. Re-enable together with the binding
in HudiModule.")
public void testSelectWithFilter()
{
@Language("SQL") String query = "SELECT * FROM " +
HUDI_MULTI_FG_PT_V8_MOR + " WHERE country='SG'";
@@ -103,6 +105,7 @@ public class TestHudiAlluxioCacheFileOperations
}
@Test
+ @Disabled("Asserts the exclusions of HudiCacheKeyProvider (table
properties and index definitions are re-read from storage on every query).
release-1.2.1 does not bind that provider, so Trino's default provider caches
those files and the counts no longer hold. Re-enable together with the binding
in HudiModule.")
public void testJoin()
{
@Language("SQL") String query = "SELECT t1.id, t1.name, t1.price,
t1.ts FROM " +
diff --git
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
index b5ada5741411..03cd41ee4c8d 100644
---
a/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
+++
b/hudi-trino/src/test/java/io/trino/plugin/hudi/TestHudiMemoryCacheFileOperations.java
@@ -23,6 +23,7 @@ import io.trino.testing.AbstractTestQueryFramework;
import io.trino.testing.DistributedQueryRunner;
import io.trino.testing.QueryRunner;
import org.intellij.lang.annotations.Language;
+import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.Execution;
import org.junit.jupiter.api.parallel.ExecutionMode;
@@ -40,6 +41,7 @@ import static
io.trino.plugin.hudi.util.FileOperationUtils.FileType.METADATA_TAB
import static
io.trino.plugin.hudi.util.FileOperationUtils.FileType.TABLE_PROPERTIES;
import static io.trino.testing.MultisetAssertions.assertMultisetsEqual;
import static java.util.stream.Collectors.toCollection;
+import static org.assertj.core.api.Assertions.assertThat;
@ResourceLock("HUDI_CACHE_SYSTEM")
@Execution(ExecutionMode.SAME_THREAD)
@@ -70,6 +72,7 @@ public class TestHudiMemoryCacheFileOperations
}
@Test
+ @Disabled("Asserts the exclusions of HudiCacheKeyProvider in the metadata
memory cache (table properties and index definitions are re-read from storage
on every query). release-1.2.1 does not bind that provider, so Trino's default
provider caches those files and the counts no longer hold. Re-enable together
with the binding in HudiModule.")
public void testSelectWithFilter()
{
@Language("SQL") String query = "SELECT * FROM " +
HUDI_MULTI_FG_PT_V8_MOR + " WHERE country='SG'";
@@ -93,6 +96,7 @@ public class TestHudiMemoryCacheFileOperations
}
@Test
+ @Disabled("Asserts the exclusions of HudiCacheKeyProvider in the metadata
memory cache (table properties and index definitions are re-read from storage
on every query). release-1.2.1 does not bind that provider, so Trino's default
provider caches those files and the counts no longer hold. Re-enable together
with the binding in HudiModule.")
public void testJoin()
{
@Language("SQL") String query = "SELECT t1.id, t1.name, t1.price,
t1.ts FROM " +
@@ -117,6 +121,32 @@ public class TestHudiMemoryCacheFileOperations
.build());
}
+ @Test
+ public void testReadsServedFromMemoryCache()
+ {
+ // Count-independent coverage that the memory cache serves reads,
whichever CacheKeyProvider is
+ // bound: once a query has warmed the cache, repeating it still goes
through the cache for data
+ // files (FileSystemCache.cacheInput) but opens none of them from
storage (InputFile.newInput is
+ // only issued when the cache loads a missing entry).
+ @Language("SQL") String query = "SELECT * FROM " +
HUDI_MULTI_FG_PT_V8_MOR;
+ DistributedQueryRunner queryRunner = getDistributedQueryRunner();
+ queryRunner.executeWithPlan(queryRunner.getDefaultSession(), query);
+
+ queryRunner.executeWithPlan(queryRunner.getDefaultSession(), query);
+ Multiset<FileOperation> dataOperations =
queryRunner.getSpans().stream()
+ .filter(span ->
span.getName().equals("FileSystemCache.cacheInput") ||
span.getName().equals("InputFile.newInput"))
+ .filter(span ->
!isTrinoSchemaOrPermissions(getFileLocation(span)))
+ .map(FileOperation::create)
+ .filter(operation -> operation.fileType() == DATA)
+ .collect(toCollection(HashMultiset::create));
+ assertThat(dataOperations.count(new
FileOperation("FileSystemCache.cacheInput", DATA)))
+ .as("data file reads through the memory cache")
+ .isGreaterThanOrEqualTo(1);
+ assertThat(dataOperations.count(new
FileOperation("InputFile.newInput", DATA)))
+ .as("data files opened from storage after the cache was
warmed")
+ .isZero();
+ }
+
private void assertFileSystemAccesses(@Language("SQL") String query,
Multiset<FileOperation> expectedCacheAccesses)
{
DistributedQueryRunner queryRunner = getDistributedQueryRunner();