yuqi1129 commented on PR #13606: URL: https://github.com/apache/gravitino/pull/13606#issuecomment-6054099530
Thanks for the review. Pushed follow-up commit 46ff98e9aa. It addresses the initial-load gap and the three nits: - The initial role load retains unreadable role ids on the request context. Every policy evaluation for that request fails closed: allow is false, deny and hasDenyPolicy are true. A fresh request can recover once the role store becomes readable. - Role policy generation advances atomically with max, so concurrent list workers cannot move it backwards. - Reloads reuse the immutable bound-role list, and the maxRoleClearGenerations Javadoc now describes initialization and reads accurately. - Tests cover initial-load read failure through all three entry points, recovery in a fresh request, reload RuntimeException, hasDenyPolicy reload-budget exhaustion, and generation updates across workers. On partial loads: I agree that hasDenyPolicy cannot prove the absence of a deny from incomplete policies. This is a separate pre-existing issue and should get a dedicated fix rather than be folded into #13612 (single-flight). The backoff cache expires independently of the partial policies, so checking only that cache would leave the gap open. A fix needs reliable completeness tracking, including backoff expiry and concurrent complete/partial replacements. This patch preserves the existing per-object partial-load behavior. I will track completeness handling in a separate issue and follow-up PR. Validation: 397 authorization tests passed (server-common 209, core 85, server 103); formatting passed for both changed modules. Full-repository spotlessApply requires JDK 24 for an unrelated Trino integration module. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
