[ 
https://issues.apache.org/jira/browse/IMPALA-10130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17188891#comment-17188891
 ] 

Quanlong Huang commented on IMPALA-10130:
-----------------------------------------

I think this issue only exists on the 3.x branch, because the Sentry support 
has been removed in Impala 4.0. Note that authPolicy is only used by Sentry 
integration. For Ranger integration, each coordinator has an embedded ranger 
plugin to poll the policies directly from ranger.

> 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: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to