From 0a485b4dbd3e2e3e361cb58c120dd4062d514326 Mon Sep 17 00:00:00 2001 From: fanng Date: Thu, 26 Dec 2024 14:34:14 +0800 Subject: [PATCH] fix gccs --- .../credential/ADLSTokenCredential.java | 2 +- .../credential/CatalogCredentialManager.java | 25 +++++++++---------- .../gravitino/credential/CredentialCache.java | 18 ++++++++++--- .../credential/CredentialCacheKey.java | 4 +-- .../credential/CredentialContext.java | 1 - .../CredentialOperationDispatcher.java | 2 +- .../credential/config/CredentialConfig.java | 16 ++++++------ .../credential/TestCredentialCacheKey.java | 2 +- 8 files changed, 39 insertions(+), 31 deletions(-) diff --git a/api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java b/api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java index 5c7c93d08ee..25c83c2f7cc 100644 --- a/api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java +++ b/api/src/main/java/org/apache/gravitino/credential/ADLSTokenCredential.java @@ -28,7 +28,7 @@ public class ADLSTokenCredential implements Credential { /** ADLS SAS token credential type. */ - public static final String ADLS_SAS_TOKEN_CREDENTIAL_TYPE = "adls-token"; + public static final String ADLS_SAS_TOKEN_CREDENTIAL_TYPE = "adls-sas-token"; /** ADLS base domain */ public static final String ADLS_DOMAIN = "dfs.core.windows.net"; /** ADLS storage account name */ diff --git a/core/src/main/java/org/apache/gravitino/credential/CatalogCredentialManager.java b/core/src/main/java/org/apache/gravitino/credential/CatalogCredentialManager.java index af5a2e8f3b4..53a23a7bd1d 100644 --- a/core/src/main/java/org/apache/gravitino/credential/CatalogCredentialManager.java +++ b/core/src/main/java/org/apache/gravitino/credential/CatalogCredentialManager.java @@ -19,10 +19,10 @@ package org.apache.gravitino.credential; +import com.google.common.base.Preconditions; import java.io.Closeable; import java.io.IOException; import java.util.Map; -import org.apache.gravitino.exceptions.NoSuchCredentialException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -34,7 +34,7 @@ public class CatalogCredentialManager implements Closeable { private static final Logger LOG = LoggerFactory.getLogger(CatalogCredentialManager.class); - private CredentialCache cacheManager; + private CredentialCache credentialCache; private final String catalogName; private final Map credentialProviders; @@ -42,13 +42,13 @@ public class CatalogCredentialManager implements Closeable { public CatalogCredentialManager(String catalogName, Map catalogProperties) { this.catalogName = catalogName; this.credentialProviders = CredentialUtils.loadCredentialProviders(catalogProperties); - this.cacheManager = new CredentialCache(); - cacheManager.initialize(catalogProperties); + this.credentialCache = new CredentialCache(); + credentialCache.initialize(catalogProperties); } - public Credential getCredential(String credentialType, CredentialContext context) { + public Credential doGetCredential(String credentialType, CredentialContext context) { CredentialCacheKey credentialCacheKey = new CredentialCacheKey(credentialType, context); - return cacheManager.getCredential(credentialCacheKey, cacheKey -> getCredential(cacheKey)); + return credentialCache.getCredential(credentialCacheKey, cacheKey -> doGetCredential(cacheKey)); } @Override @@ -69,14 +69,13 @@ public void close() { }); } - private Credential getCredential(CredentialCacheKey credentialCacheKey) { + private Credential doGetCredential(CredentialCacheKey credentialCacheKey) { String credentialType = credentialCacheKey.getCredentialType(); CredentialContext context = credentialCacheKey.getCredentialContext(); - LOG.info("try get credential, credential type: {}, context: {}", credentialType, context); - CredentialProvider credentialProvider = credentialProviders.get(credentialType); - if (credentialProvider == null) { - throw new NoSuchCredentialException("No such credential: %s", credentialType); - } - return credentialProvider.getCredential(context); + LOG.debug("try get credential, credential type: {}, context: {}", credentialType, context); + Preconditions.checkState( + credentialProviders.containsKey(credentialType), + String.format("Credential %s not found", credentialType)); + return credentialProviders.get(credentialType).getCredential(context); } } diff --git a/core/src/main/java/org/apache/gravitino/credential/CredentialCache.java b/core/src/main/java/org/apache/gravitino/credential/CredentialCache.java index d9f939a699f..b94589a9f4f 100644 --- a/core/src/main/java/org/apache/gravitino/credential/CredentialCache.java +++ b/core/src/main/java/org/apache/gravitino/credential/CredentialCache.java @@ -22,6 +22,8 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import com.github.benmanes.caffeine.cache.Expiry; +import java.io.Closeable; +import java.io.IOException; import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.function.Function; @@ -31,7 +33,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class CredentialCache { +public class CredentialCache implements Closeable { private static final Logger LOG = LoggerFactory.getLogger(CredentialCache.class); @@ -84,8 +86,8 @@ public long expireAfterRead( public void initialize(Map catalogProperties) { CredentialConfig credentialConfig = new CredentialConfig(catalogProperties); - long cache_size = credentialConfig.get(CredentialConfig.CREDENTIAL_MAX_CACHE_SIZE); - long cache_time = credentialConfig.get(CredentialConfig.CREDENTIAL_MAX_CACHE_TIME); + long cache_size = credentialConfig.get(CredentialConfig.CREDENTIAL_CACHE_MAZ_SIZE); + long cache_time = credentialConfig.get(CredentialConfig.CREDENTIAL_CACHE_MAX_TIME_IN_SECS); this.credentialCache = Caffeine.newBuilder() @@ -93,7 +95,7 @@ public void initialize(Map catalogProperties) { .maximumSize(cache_size) .removalListener( (cacheKey, credential, c) -> { - LOG.info("credential expire {}.", cacheKey); + LOG.debug("credential expire {}.", cacheKey); }) .build(); } @@ -101,4 +103,12 @@ public void initialize(Map catalogProperties) { public Credential getCredential(T cacheKey, Function credentialSupplier) { return credentialCache.get(cacheKey, key -> credentialSupplier.apply(cacheKey)); } + + @Override + public void close() throws IOException { + if (credentialCache != null) { + credentialCache.invalidateAll(); + credentialCache = null; + } + } } diff --git a/core/src/main/java/org/apache/gravitino/credential/CredentialCacheKey.java b/core/src/main/java/org/apache/gravitino/credential/CredentialCacheKey.java index 92645dd360c..1d0d8f7b3b6 100644 --- a/core/src/main/java/org/apache/gravitino/credential/CredentialCacheKey.java +++ b/core/src/main/java/org/apache/gravitino/credential/CredentialCacheKey.java @@ -25,8 +25,8 @@ @Getter public class CredentialCacheKey { - private String credentialType; - private CredentialContext credentialContext; + private final String credentialType; + private final CredentialContext credentialContext; public CredentialCacheKey(String credentialType, CredentialContext credentialContext) { this.credentialType = credentialType; diff --git a/core/src/main/java/org/apache/gravitino/credential/CredentialContext.java b/core/src/main/java/org/apache/gravitino/credential/CredentialContext.java index 8a1bc2862a5..6e82efea0f1 100644 --- a/core/src/main/java/org/apache/gravitino/credential/CredentialContext.java +++ b/core/src/main/java/org/apache/gravitino/credential/CredentialContext.java @@ -21,7 +21,6 @@ /** Contains credential context information to get credential from a credential provider. */ public interface CredentialContext { - /** * Providing the username. * diff --git a/core/src/main/java/org/apache/gravitino/credential/CredentialOperationDispatcher.java b/core/src/main/java/org/apache/gravitino/credential/CredentialOperationDispatcher.java index 2ec76aeb4ad..ab0d2255714 100644 --- a/core/src/main/java/org/apache/gravitino/credential/CredentialOperationDispatcher.java +++ b/core/src/main/java/org/apache/gravitino/credential/CredentialOperationDispatcher.java @@ -67,7 +67,7 @@ private List getCredentials( entry -> baseCatalog .catalogCredentialManager() - .getCredential(entry.getKey(), entry.getValue())) + .doGetCredential(entry.getKey(), entry.getValue())) .filter(Objects::nonNull) .collect(Collectors.toList()); } diff --git a/core/src/main/java/org/apache/gravitino/credential/config/CredentialConfig.java b/core/src/main/java/org/apache/gravitino/credential/config/CredentialConfig.java index 5c61e314e47..f43adb2b4a4 100644 --- a/core/src/main/java/org/apache/gravitino/credential/config/CredentialConfig.java +++ b/core/src/main/java/org/apache/gravitino/credential/config/CredentialConfig.java @@ -30,8 +30,8 @@ public class CredentialConfig extends Config { - private static final long DEFAULT_MAX_CREDENTIAL_CACHE_SIZE = 10_000L; - private static final long DEFAULT_MAX_CREDENTIAL_CACHE_TIME = 300; + private static final long DEFAULT_CREDENTIAL_CACHE_MAX_SIZE = 10_000L; + private static final long DEFAULT_CREDENTIAL_CACHE_MAX_TIME = 300; public static final Map> CREDENTIAL_PROPERTY_ENTRIES = new ImmutableMap.Builder>() @@ -52,7 +52,7 @@ public class CredentialConfig extends Config { "Max cache time for the credential.", false /* required */, false /* immutable */, - DEFAULT_MAX_CREDENTIAL_CACHE_TIME /* default value */, + DEFAULT_CREDENTIAL_CACHE_MAX_TIME /* default value */, false /* hidden */, false /* reserved */)) .put( @@ -62,24 +62,24 @@ public class CredentialConfig extends Config { "Max size for the credential cache.", false /* required */, false /* immutable */, - DEFAULT_MAX_CREDENTIAL_CACHE_SIZE /* default value */, + DEFAULT_CREDENTIAL_CACHE_MAX_SIZE /* default value */, false /* hidden */, false /* reserved */)) .build(); - public static final ConfigEntry CREDENTIAL_MAX_CACHE_TIME = + public static final ConfigEntry CREDENTIAL_CACHE_MAX_TIME_IN_SECS = new ConfigBuilder(CredentialConstants.CREDENTIAL_CACHE_MAX_TIME_IN_SECS) .doc("Max cache time for the credential.") .version(ConfigConstants.VERSION_0_8_0) .longConf() - .createWithDefault(DEFAULT_MAX_CREDENTIAL_CACHE_TIME); + .createWithDefault(DEFAULT_CREDENTIAL_CACHE_MAX_TIME); - public static final ConfigEntry CREDENTIAL_MAX_CACHE_SIZE = + public static final ConfigEntry CREDENTIAL_CACHE_MAZ_SIZE = new ConfigBuilder(CredentialConstants.CREDENTIAL_CACHE_MAX_SIZE) .doc("Max cache size for the credential.") .version(ConfigConstants.VERSION_0_8_0) .longConf() - .createWithDefault(DEFAULT_MAX_CREDENTIAL_CACHE_SIZE); + .createWithDefault(DEFAULT_CREDENTIAL_CACHE_MAX_SIZE); public CredentialConfig(Map properties) { super(false); diff --git a/core/src/test/java/org/apache/gravitino/credential/TestCredentialCacheKey.java b/core/src/test/java/org/apache/gravitino/credential/TestCredentialCacheKey.java index 67700439900..eda3ab7cbbe 100644 --- a/core/src/test/java/org/apache/gravitino/credential/TestCredentialCacheKey.java +++ b/core/src/test/java/org/apache/gravitino/credential/TestCredentialCacheKey.java @@ -27,7 +27,7 @@ public class TestCredentialCacheKey { @Test - void testCredentialCache() { + void testCredentialCacheKey() { PathBasedCredentialContext context1 = new PathBasedCredentialContext("user1", ImmutableSet.of("path1"), ImmutableSet.of("path2"));