You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues-all@impala.apache.org by "Abhishek Rawat (Jira)" <ji...@apache.org> on 2020/09/01 23:45:00 UTC
[jira] [Updated] (IMPALA-10130) Catalog restart doesn't invalidate
authPolicy cache in local catalog
[ https://issues.apache.org/jira/browse/IMPALA-10130?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Abhishek Rawat updated IMPALA-10130:
------------------------------------
Description:
When the catalog service is restarted, LocalCatalog detects it in the topic update due to the change in its service id and invalidates its cache contents. However, it looks like it doesn't invalidate its existing authPolicy cache contents.
{code:java}
private void witnessCatalogServiceId(TUniqueId serviceId) {
synchronized (catalogServiceIdLock_) {
if (!catalogServiceId_.equals(serviceId)) {
if (!catalogServiceId_.equals(Catalog.INITIAL_CATALOG_SERVICE_ID)) {
LOG.warn("Detected catalog service restart: service ID changed from " +
"{} to {}. Invalidating all cached metadata on this coordinator.",
catalogServiceId_, serviceId);
}
catalogServiceId_ = serviceId;
cache_.invalidateAll();
// Clear cached items from the previous catalogd instance. Otherwise, we'll
// ignore new updates from the new catalogd instance since they have lower
// versions.
hdfsCachePools_.clear();
// TODO(todd): we probably need to invalidate the auth policy too.
// we are probably better off detecting this at a higher level and
// reinstantiating the metaprovider entirely, similar to how ImpaladCatalog
// handles this.
// TODO(todd): slight race here: a concurrent request from the old catalog
// could theoretically be just about to write something back into the cache
// after we do the above invalidate. Maybe we would be better off replacing
// the whole cache object, or doing a soft barrier here to wait for any
// concurrent cache accessors to cycle out. Another option is to associate
// the catalog service ID as part of all of the cache keys.
//
// This is quite unlikely to be an issue in practice, so deferring it to later
// clean-up.
}
}
}
{code}
If the older authpolicy is not cleared above, it is possible that when the principle was added into the cache it was ignored since its catalog version was higher as seen below:
{code:java}
public synchronized void addPrincipal(Principal principal) {
Principal existingPrincipal = getPrincipal(principal.getName(),
principal.getPrincipalType());
// There is already a newer version of this principal in the catalog, ignore
// just return.
if (existingPrincipal != null &&
existingPrincipal.getCatalogVersion() >= principal.getCatalogVersion()) return;
{code}
When the update tries to add the privilege associated with the principle above, it looks up the principal using id instead of name and if there is a id mismatch it throws error.
{code:java}
/**
* Adds a new privilege to the policy mapping to the principal specified by the
* principal ID in the privilege. Throws a CatalogException no principal with a
* corresponding ID existing in the catalog.
*/
public synchronized void addPrivilege(PrincipalPrivilege privilege)
throws CatalogException {
if (LOG.isTraceEnabled()) {
LOG.trace("Adding privilege: " + privilege.getName() + " " +
Principal.toString(privilege.getPrincipalType()).toLowerCase() +
" ID: " + privilege.getPrincipalId());
}
Principal principal = getPrincipal(privilege.getPrincipalId(),
privilege.getPrincipalType());
if (principal == null) {
throw new CatalogException(String.format("Error adding privilege: %s. %s ID " +
"'%d' does not exist.", privilege.getName(),
Principal.toString(privilege.getPrincipalType()), privilege.getPrincipalId()));
}{code}
The legacy catalog mode doesn't have this issue because the whole ImpaladCatalog instance is re-created when detecting catalogd restarts:
{code:java}
@Override
TUpdateCatalogCacheResponse updateCatalogCache(TUpdateCatalogCacheRequest req)
throws CatalogException, TException {
ImpaladCatalog catalog = catalog_.get();
if (req.is_delta) return catalog.updateCatalog(req);
// If this is not a delta, this update should replace the current
// Catalog contents so create a new catalog and populate it.
catalog = createNewCatalog();
TUpdateCatalogCacheResponse response = catalog.updateCatalog(req);
// Now that the catalog has been updated, replace the reference to
// catalog_. This ensures that clients don't see the catalog
// disappear. The catalog is guaranteed to be ready since updateCatalog() has a
// postcondition of isReady() == true.
catalog_.set(catalog);
return response;
}
{code}
was:
When the catalog service is restarted, LocalCatalog detects it in the topic update due to the change in its service id and invalidates its cache contents. However, it looks like it doesn't invalidate its existing authPolicy cache contents.
{code:java}
private void witnessCatalogServiceId(TUniqueId serviceId) {
synchronized (catalogServiceIdLock_) {
if (!catalogServiceId_.equals(serviceId)) {
if (!catalogServiceId_.equals(Catalog.INITIAL_CATALOG_SERVICE_ID)) {
LOG.warn("Detected catalog service restart: service ID changed from " +
"{} to {}. Invalidating all cached metadata on this coordinator.",
catalogServiceId_, serviceId);
}
catalogServiceId_ = serviceId;
cache_.invalidateAll();
// Clear cached items from the previous catalogd instance. Otherwise, we'll
// ignore new updates from the new catalogd instance since they have lower
// versions.
hdfsCachePools_.clear();
// TODO(todd): we probably need to invalidate the auth policy too.
// we are probably better off detecting this at a higher level and
// reinstantiating the metaprovider entirely, similar to how ImpaladCatalog
// handles this.
// TODO(todd): slight race here: a concurrent request from the old catalog
// could theoretically be just about to write something back into the cache
// after we do the above invalidate. Maybe we would be better off replacing
// the whole cache object, or doing a soft barrier here to wait for any
// concurrent cache accessors to cycle out. Another option is to associate
// the catalog service ID as part of all of the cache keys.
//
// This is quite unlikely to be an issue in practice, so deferring it to later
// clean-up.
}
}
}
{code}
If the older authpolicy is not cleared above, it is possible that when the principle was added into the cache it was ignored since its catalog version was higher as seen below:
{code:java}
public synchronized void addPrincipal(Principal principal) {
Principal existingPrincipal = getPrincipal(principal.getName(),
principal.getPrincipalType());
// There is already a newer version of this principal in the catalog, ignore
// just return.
if (existingPrincipal != null &&
existingPrincipal.getCatalogVersion() >= principal.getCatalogVersion()) return;
{code}
When the update tries to add the privilege associated with the principle above, it looks up the principal using id instead of name and if there is a id mismatch it throws error.
{code:java}
/**
* Adds a new privilege to the policy mapping to the principal specified by the
* principal ID in the privilege. Throws a CatalogException no principal with a
* corresponding ID existing in the catalog.
*/
public synchronized void addPrivilege(PrincipalPrivilege privilege)
throws CatalogException {
if (LOG.isTraceEnabled()) {
LOG.trace("Adding privilege: " + privilege.getName() + " " +
Principal.toString(privilege.getPrincipalType()).toLowerCase() +
" ID: " + privilege.getPrincipalId());
}
Principal principal = getPrincipal(privilege.getPrincipalId(),
privilege.getPrincipalType());
if (principal == null) {
throw new CatalogException(String.format("Error adding privilege: %s. %s ID " +
"'%d' does not exist.", privilege.getName(),
Principal.toString(privilege.getPrincipalType()), privilege.getPrincipalId()));
}{code}
The legacy catalog mode doesn't have this issue because the whole ImpaladCatalog instance is re-created when detecting catalogd restarts:
@Override
TUpdateCatalogCacheResponse updateCatalogCache(TUpdateCatalogCacheRequest req) throws CatalogException, TException {
ImpaladCatalog catalog = catalog_.get(); if (req.is_delta) return catalog.updateCatalog(req); // If this is not a delta, this update should replace the current // Catalog contents so create a new catalog and populate it. catalog = createNewCatalog();
TUpdateCatalogCacheResponse response = catalog.updateCatalog(req); // Now that the catalog has been updated, replace the reference to // catalog_. This ensures that clients don't see the catalog // disappear. The catalog is guaranteed to be ready since updateCatalog() has a // postcondition of isReady() == true. catalog_.set(catalog); return response;
}
> Catalog restart doesn't invalidate authPolicy cache in local catalog
> --------------------------------------------------------------------
>
> Key: IMPALA-10130
> URL: https://issues.apache.org/jira/browse/IMPALA-10130
> Project: IMPALA
> Issue Type: Bug
> Components: Catalog
> Affects Versions: Impala 2.0, Impala 3.0, Impala 4.0
> Reporter: Abhishek Rawat
> Priority: Major
>
> When the catalog service is restarted, LocalCatalog detects it in the topic update due to the change in its service id and invalidates its cache contents. However, it looks like it doesn't invalidate its existing authPolicy cache contents.
> {code:java}
> private void witnessCatalogServiceId(TUniqueId serviceId) {
> synchronized (catalogServiceIdLock_) {
> if (!catalogServiceId_.equals(serviceId)) {
> if (!catalogServiceId_.equals(Catalog.INITIAL_CATALOG_SERVICE_ID)) {
> LOG.warn("Detected catalog service restart: service ID changed from " +
> "{} to {}. Invalidating all cached metadata on this coordinator.",
> catalogServiceId_, serviceId);
> }
> catalogServiceId_ = serviceId;
> cache_.invalidateAll();
> // Clear cached items from the previous catalogd instance. Otherwise, we'll
> // ignore new updates from the new catalogd instance since they have lower
> // versions.
> hdfsCachePools_.clear();
> // TODO(todd): we probably need to invalidate the auth policy too.
> // we are probably better off detecting this at a higher level and
> // reinstantiating the metaprovider entirely, similar to how ImpaladCatalog
> // handles this.
> // TODO(todd): slight race here: a concurrent request from the old catalog
> // could theoretically be just about to write something back into the cache
> // after we do the above invalidate. Maybe we would be better off replacing
> // the whole cache object, or doing a soft barrier here to wait for any
> // concurrent cache accessors to cycle out. Another option is to associate
> // the catalog service ID as part of all of the cache keys.
> //
> // This is quite unlikely to be an issue in practice, so deferring it to later
> // clean-up.
> }
> }
> }
> {code}
> If the older authpolicy is not cleared above, it is possible that when the principle was added into the cache it was ignored since its catalog version was higher as seen below:
> {code:java}
> public synchronized void addPrincipal(Principal principal) {
> Principal existingPrincipal = getPrincipal(principal.getName(),
> principal.getPrincipalType());
> // There is already a newer version of this principal in the catalog, ignore
> // just return.
> if (existingPrincipal != null &&
> existingPrincipal.getCatalogVersion() >= principal.getCatalogVersion()) return;
> {code}
> When the update tries to add the privilege associated with the principle above, it looks up the principal using id instead of name and if there is a id mismatch it throws error.
> {code:java}
> /**
> * Adds a new privilege to the policy mapping to the principal specified by the
> * principal ID in the privilege. Throws a CatalogException no principal with a
> * corresponding ID existing in the catalog.
> */
> public synchronized void addPrivilege(PrincipalPrivilege privilege)
> throws CatalogException {
> if (LOG.isTraceEnabled()) {
> LOG.trace("Adding privilege: " + privilege.getName() + " " +
> Principal.toString(privilege.getPrincipalType()).toLowerCase() +
> " ID: " + privilege.getPrincipalId());
> }
> Principal principal = getPrincipal(privilege.getPrincipalId(),
> privilege.getPrincipalType());
> if (principal == null) {
> throw new CatalogException(String.format("Error adding privilege: %s. %s ID " +
> "'%d' does not exist.", privilege.getName(),
> Principal.toString(privilege.getPrincipalType()), privilege.getPrincipalId()));
> }{code}
>
> The legacy catalog mode doesn't have this issue because the whole ImpaladCatalog instance is re-created when detecting catalogd restarts:
> {code:java}
> @Override
> TUpdateCatalogCacheResponse updateCatalogCache(TUpdateCatalogCacheRequest req)
> throws CatalogException, TException {
> ImpaladCatalog catalog = catalog_.get();
> if (req.is_delta) return catalog.updateCatalog(req);
> // If this is not a delta, this update should replace the current
> // Catalog contents so create a new catalog and populate it.
> catalog = createNewCatalog();
> TUpdateCatalogCacheResponse response = catalog.updateCatalog(req);
> // Now that the catalog has been updated, replace the reference to
> // catalog_. This ensures that clients don't see the catalog
> // disappear. The catalog is guaranteed to be ready since updateCatalog() has a
> // postcondition of isReady() == true.
> catalog_.set(catalog);
> return response;
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org