Skip to content

Add PolarisDiagnostics field to BaseMetaStoreManager #2381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 26, 2025
Merged

Add PolarisDiagnostics field to BaseMetaStoreManager #2381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cef397d
Add PolarisDiagnostics field to BaseMetaStoreManager
XN137 Aug 19, 2025
0a024d1
review: remove field from TransactionalMetaStoreManagerImpl
XN137 Aug 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...main/java/org/apache/polaris/persistence/relational/jdbc/JdbcMetaStoreManagerFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ protected PrincipalSecretsGenerator secretsGenerator(
}

protected PolarisMetaStoreManager createNewMetaStoreManager() {
return new AtomicOperationMetaStoreManager(clock);
return new AtomicOperationMetaStoreManager(clock, diagnostics);
}

private void initializeForRealm(
Expand Down
3 changes: 2 additions & 1 deletion ...is/persistence/relational/jdbc/AtomicMetastoreManagerWithJdbcBasePersistenceImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ protected PolarisTestMetaStoreManager createPolarisTestMetaStoreManager() {
Mockito.mock(),
realmContext.getRealmIdentifier(),
schemaVersion);
AtomicOperationMetaStoreManager metaStoreManager = new AtomicOperationMetaStoreManager(clock);
AtomicOperationMetaStoreManager metaStoreManager =
new AtomicOperationMetaStoreManager(clock, diagServices);
PolarisCallContext callCtx =
new PolarisCallContext(realmContext, basePersistence, diagServices);
return new PolarisTestMetaStoreManager(metaStoreManager, callCtx);
Expand Down
102 changes: 41 additions & 61 deletions ...re/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.function.Function;
import java.util.stream.Collectors;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.entity.AsyncTaskType;
import org.apache.polaris.core.entity.EntityNameLookupRecord;
import org.apache.polaris.core.entity.LocationBasedEntity;
Expand Down Expand Up @@ -86,7 +87,8 @@ public class AtomicOperationMetaStoreManager extends BaseMetaStoreManager {

private final Clock clock;

public AtomicOperationMetaStoreManager(Clock clock) {
public AtomicOperationMetaStoreManager(Clock clock, PolarisDiagnostics diagnostics) {
super(diagnostics);
this.clock = clock;
}

Expand Down Expand Up @@ -174,11 +176,11 @@ private void dropEntity(
@Nonnull PolarisBaseEntity entity) {

// validate the entity type and subtype
callCtx.getDiagServices().checkNotNull(entity, "unexpected_null_dpo");
callCtx.getDiagServices().checkNotNull(entity.getName(), "unexpected_null_name");
getDiagnostics().checkNotNull(entity, "unexpected_null_dpo");
getDiagnostics().checkNotNull(entity.getName(), "unexpected_null_name");

// creation timestamp must be filled
callCtx.getDiagServices().check(entity.getDropTimestamp() == 0, "already_dropped");
getDiagnostics().check(entity.getDropTimestamp() == 0, "already_dropped");

// Remove the main entity itself first-thing; once its id no longer resolves successfully
// it will be pruned out of any grant-record lookups anyways.
Expand Down Expand Up @@ -281,13 +283,12 @@ private void dropEntity(
@Nonnull PolarisPrivilege priv) {

// validate non null arguments
callCtx.getDiagServices().checkNotNull(securable, "unexpected_null_securable");
callCtx.getDiagServices().checkNotNull(grantee, "unexpected_null_grantee");
callCtx.getDiagServices().checkNotNull(priv, "unexpected_null_priv");
getDiagnostics().checkNotNull(securable, "unexpected_null_securable");
getDiagnostics().checkNotNull(grantee, "unexpected_null_grantee");
getDiagnostics().checkNotNull(priv, "unexpected_null_priv");

// ensure that this entity is indeed a grantee like entity
callCtx
.getDiagServices()
getDiagnostics()
.check(grantee.getType().isGrantee(), "entity_must_be_grantee", "entity={}", grantee);

// create new grant record
Expand All @@ -306,9 +307,7 @@ private void dropEntity(
// version
PolarisBaseEntity granteeEntity =
ms.lookupEntity(callCtx, grantee.getCatalogId(), grantee.getId(), grantee.getTypeCode());
callCtx
.getDiagServices()
.checkNotNull(granteeEntity, "grantee_not_found", "grantee={}", grantee);
getDiagnostics().checkNotNull(granteeEntity, "grantee_not_found", "grantee={}", grantee);
// grants have changed, we need to bump-up the grants version
PolarisBaseEntity updatedGranteeEntity =
granteeEntity.withGrantRecordsVersion(granteeEntity.getGrantRecordsVersion() + 1);
Expand All @@ -319,8 +318,7 @@ private void dropEntity(
PolarisBaseEntity securableEntity =
ms.lookupEntity(
callCtx, securable.getCatalogId(), securable.getId(), securable.getTypeCode());
callCtx
.getDiagServices()
getDiagnostics()
.checkNotNull(securableEntity, "securable_not_found", "securable={}", securable);
// grants have changed, we need to bump-up the grants version
PolarisBaseEntity updatedSecurableEntity =
Expand Down Expand Up @@ -356,8 +354,7 @@ private void revokeGrantRecord(
@Nonnull PolarisGrantRecord grantRecord) {

// validate securable
callCtx
.getDiagServices()
getDiagnostics()
.check(
securable.getCatalogId() == grantRecord.getSecurableCatalogId()
&& securable.getId() == grantRecord.getSecurableId(),
Expand All @@ -367,8 +364,7 @@ private void revokeGrantRecord(
grantRecord);

// validate grantee
callCtx
.getDiagServices()
getDiagnostics()
.check(
grantee.getCatalogId() == grantRecord.getGranteeCatalogId()
&& grantee.getId() == grantRecord.getGranteeId(),
Expand All @@ -378,18 +374,15 @@ private void revokeGrantRecord(
grantRecord);

// ensure the grantee is really a grantee
callCtx
.getDiagServices()
.check(grantee.getType().isGrantee(), "not_a_grantee", "grantee={}", grantee);
getDiagnostics().check(grantee.getType().isGrantee(), "not_a_grantee", "grantee={}", grantee);

// remove that grant
ms.deleteFromGrantRecords(callCtx, grantRecord);

// load the grantee and increment its grants version
PolarisBaseEntity refreshGrantee =
ms.lookupEntity(callCtx, grantee.getCatalogId(), grantee.getId(), grantee.getTypeCode());
callCtx
.getDiagServices()
getDiagnostics()
.checkNotNull(
refreshGrantee, "missing_grantee", "grantRecord={} grantee={}", grantRecord, grantee);
// grants have changed, we need to bump-up the grants version
Expand All @@ -402,8 +395,7 @@ private void revokeGrantRecord(
PolarisBaseEntity refreshSecurable =
ms.lookupEntity(
callCtx, securable.getCatalogId(), securable.getId(), securable.getTypeCode());
callCtx
.getDiagServices()
getDiagnostics()
.checkNotNull(
refreshSecurable,
"missing_securable",
Expand All @@ -430,7 +422,7 @@ private void revokeGrantRecord(
BasePersistence ms = callCtx.getMetaStore();

// validate input
callCtx.getDiagServices().checkNotNull(catalog, "unexpected_null_catalog");
getDiagnostics().checkNotNull(catalog, "unexpected_null_catalog");

Map<String, String> internalProp = catalog.getInternalPropertiesAsMap();
String integrationIdentifierOrId =
Expand Down Expand Up @@ -462,8 +454,7 @@ private void revokeGrantRecord(
// if found, probably a retry, simply return the previously created catalog
if (refreshCatalog != null) {
// if found, ensure it is indeed a catalog
callCtx
.getDiagServices()
getDiagnostics()
.check(
refreshCatalog.getTypeCode() == PolarisEntityType.CATALOG.getCode(),
"not_a_catalog",
Expand All @@ -480,8 +471,7 @@ private void revokeGrantRecord(
PolarisEntityConstants.getNameOfCatalogAdminRole());

// if found, ensure not null
callCtx
.getDiagServices()
getDiagnostics()
.checkNotNull(
catalogAdminRole, "catalog_admin_role_not_found", "catalog={}", refreshCatalog);

Expand Down Expand Up @@ -532,16 +522,15 @@ private void revokeGrantRecord(
PolarisEntityConstants.getRootEntityId(),
PolarisEntityType.PRINCIPAL_ROLE.getCode(),
PolarisEntityConstants.getNameOfPrincipalServiceAdminRole());
callCtx.getDiagServices().checkNotNull(serviceAdminRole, "missing_service_admin_role");
getDiagnostics().checkNotNull(serviceAdminRole, "missing_service_admin_role");
this.persistNewGrantRecord(
callCtx, ms, adminRole, serviceAdminRole, PolarisPrivilege.CATALOG_ROLE_USAGE);
} else {
// grant to each principal role usage on its catalog_admin role
for (PolarisEntityCore principalRole : principalRoles) {
// validate not null and really a principal role
callCtx.getDiagServices().checkNotNull(principalRole, "null principal role");
callCtx
.getDiagServices()
getDiagnostics().checkNotNull(principalRole, "null principal role");
getDiagnostics()
.check(
principalRole.getTypeCode() == PolarisEntityType.PRINCIPAL_ROLE.getCode(),
"not_principal_role",
Expand Down Expand Up @@ -764,7 +753,7 @@ private void revokeGrantRecord(
BasePersistence ms = callCtx.getMetaStore();

// validate input
callCtx.getDiagServices().checkNotNull(principal, "unexpected_null_principal");
getDiagnostics().checkNotNull(principal, "unexpected_null_principal");

// check if that catalog has already been created
PolarisBaseEntity refreshPrincipal =
Expand All @@ -777,8 +766,7 @@ private void revokeGrantRecord(
// there is no concurrency conflict for something else creating a principal of this same id.
if (refreshPrincipal != null) {
// if found, ensure it is indeed a principal
callCtx
.getDiagServices()
getDiagnostics()
.check(
principal.getTypeCode() == PolarisEntityType.PRINCIPAL.getCode(),
"not_a_principal",
Expand All @@ -792,16 +780,14 @@ private void revokeGrantRecord(
String clientId = properties.get(PolarisEntityConstants.getClientIdPropertyName());

// should not be null
callCtx
.getDiagServices()
getDiagnostics()
.checkNotNull(
clientId,
"null_client_id",
"properties={}",
refreshPrincipal.getInternalProperties());
// ensure non null and non empty
callCtx
.getDiagServices()
getDiagnostics()
.check(
!clientId.isEmpty(),
"empty_client_id",
Expand All @@ -813,8 +799,7 @@ private void revokeGrantRecord(
((IntegrationPersistence) ms).loadPrincipalSecrets(callCtx, clientId);

// should not be null
callCtx
.getDiagServices()
getDiagnostics()
.checkNotNull(
principalSecrets,
"missing_principal_secrets",
Expand Down Expand Up @@ -937,10 +922,10 @@ private void revokeGrantRecord(
BasePersistence ms = callCtx.getMetaStore();

// entity cannot be null
callCtx.getDiagServices().checkNotNull(entity, "unexpected_null_entity");
getDiagnostics().checkNotNull(entity, "unexpected_null_entity");

// entity name must be specified
callCtx.getDiagServices().checkNotNull(entity.getName(), "unexpected_null_entity_name");
getDiagnostics().checkNotNull(entity.getName(), "unexpected_null_entity_name");

// TODO: Use post-validation to enforce consistent view against catalogPath. In the
// meantime, happens-before ordering semantics aren't guaranteed during high-concurrency
Expand Down Expand Up @@ -994,7 +979,7 @@ private void revokeGrantRecord(
BasePersistence ms = callCtx.getMetaStore();

// entity cannot be null
callCtx.getDiagServices().checkNotNull(entity, "unexpected_null_entity");
getDiagnostics().checkNotNull(entity, "unexpected_null_entity");
// persist this entity after changing it. This will update the version and update the last
// updated time. Because the entity version is changed, we will update the change tracking table
try {
Expand All @@ -1018,7 +1003,7 @@ private void revokeGrantRecord(
BasePersistence ms = callCtx.getMetaStore();

// ensure that the entities list is not null
callCtx.getDiagServices().checkNotNull(entities, "unexpected_null_entities");
getDiagnostics().checkNotNull(entities, "unexpected_null_entities");

List<PolarisBaseEntity> updatedEntities = new ArrayList<>(entities.size());
List<PolarisBaseEntity> originalEntities = new ArrayList<>(entities.size());
Expand Down Expand Up @@ -1059,13 +1044,12 @@ private void revokeGrantRecord(
BasePersistence ms = callCtx.getMetaStore();

// entity and new name cannot be null
callCtx.getDiagServices().checkNotNull(entityToRename, "unexpected_null_entityToRename");
callCtx.getDiagServices().checkNotNull(renamedEntity, "unexpected_null_renamedEntity");
getDiagnostics().checkNotNull(entityToRename, "unexpected_null_entityToRename");
getDiagnostics().checkNotNull(renamedEntity, "unexpected_null_renamedEntity");

// if a new catalog path is specified (i.e. re-parent operation), a catalog path should be
// specified too
callCtx
.getDiagServices()
getDiagnostics()
.check(
(newCatalogPath == null) || (catalogPath != null),
"newCatalogPath_specified_without_catalogPath");
Expand Down Expand Up @@ -1159,7 +1143,7 @@ private void revokeGrantRecord(
BasePersistence ms = callCtx.getMetaStore();

// entity cannot be null
callCtx.getDiagServices().checkNotNull(entityToDrop, "unexpected_null_entity");
getDiagnostics().checkNotNull(entityToDrop, "unexpected_null_entity");

// TODO: Either document allowance of dropping entity concurrently with potentially-impacting
// changes in the parent path (e.g. race-condition revocation of grants on parent) or
Expand Down Expand Up @@ -1300,9 +1284,7 @@ private void revokeGrantRecord(
: PolarisPrivilege.PRINCIPAL_ROLE_USAGE;

// grant usage on this role to this principal
callCtx
.getDiagServices()
.check(grantee.getType().isGrantee(), "not_a_grantee", "grantee={}", grantee);
getDiagnostics().check(grantee.getType().isGrantee(), "not_a_grantee", "grantee={}", grantee);
PolarisGrantRecord grantRecord =
this.persistNewGrantRecord(callCtx, ms, role, grantee, usagePriv);
return new PrivilegeResult(grantRecord);
Expand Down Expand Up @@ -1604,8 +1586,7 @@ private void revokeGrantRecord(

// get meta store session we should be using
BasePersistence ms = callCtx.getMetaStore();
callCtx
.getDiagServices()
getDiagnostics()
.check(
!allowedReadLocations.isEmpty() || !allowedWriteLocations.isEmpty(),
"allowed_locations_to_subscope_is_required");
Expand All @@ -1627,8 +1608,7 @@ private void revokeGrantRecord(
.loadPolarisStorageIntegration(callCtx, reloadedEntity.getEntity());

// cannot be null
callCtx
.getDiagServices()
getDiagnostics()
.checkNotNull(
storageIntegration,
"storage_integration_not_exists",
Expand Down Expand Up @@ -1931,8 +1911,8 @@ Optional<Optional<String>> hasOverlappingSiblings(
@Nonnull PolarisEntityCore target,
@Nonnull PolicyEntity policy,
Map<String, String> parameters) {
callCtx.getDiagServices().checkNotNull(target, "unexpected_null_target");
callCtx.getDiagServices().checkNotNull(policy, "unexpected_null_policy");
getDiagnostics().checkNotNull(target, "unexpected_null_target");
getDiagnostics().checkNotNull(policy, "unexpected_null_policy");

PolarisPolicyMappingRecord mappingRecord =
new PolarisPolicyMappingRecord(
Expand Down
Loading