[
https://issues.apache.org/jira/browse/HDDS-16856?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16856:
------------------------------
Attachment: TestBugMC2TenantDeleteRacesUserAssignOverRpc.java
> Tenant delete racing a user assign succeeds on a non-empty tenant and leaves
> an orphan accessId that stops OM restart
> ---------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16856
> URL: https://issues.apache.org/jira/browse/HDDS-16856
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> MC-2-check-tenant-empty-under-authorizer-lock-on-delete.patch,
> TestBugMC2TenantDeleteRacesUserAssign.java,
> TestBugMC2TenantDeleteRacesUserAssignOverRpc.java
>
>
> h3. Mechanism
> {{OMTenantDeleteRequest.preExecute}} checks
> {{OMMultiTenantManager.isTenantEmpty}} (the in memory tenant cache) before it
> takes the authorizer write lock, and {{validateAndUpdateCache}} does not
> check it again since HDDS-13361 moved the check out of the apply step.
> {{OMTenantAssignUserAccessIdRequest}} holds the same lock from its
> {{preExecute}} until its apply, and the tenant cache shows the new user only
> after that apply. A delete that arrives while an assignment holds the lock
> therefore passes the check, waits for the lock (up to 1 second), deletes the
> tenant in Ranger, and is applied right after the assignment. The apply
> removes the {{tenantStateTable}} row and keeps the new
> {{tenantAccessIdTable}} row, on every OM.
> {{OMMultiTenantManagerImpl.loadTenantCacheFromDB}} treats an accessId whose
> tenant is missing as a metadata inconsistency and throws
> {{NullPointerException: OmDBTenantState should have existed for <tenant>}}
> from the constructor. {{OzoneManager.instantiateServices}} builds the manager
> at startup, on restart and on snapshot install, so each of these fails while
> multi-tenancy is enabled.
> h3. Trigger
> # An Ozone admin or a tenant admin runs {{ozone tenant user assign <user>
> --tenant <tenant>}} on a tenant with no users.
> # While that request is between its {{preExecute}} and its apply, an Ozone
> admin runs {{ozone tenant delete <tenant>}}.
> Both RPCs must hit the same tenant within about a second. No fault is needed.
> h3. Impact
> * The delete reports success for a non-empty tenant, and the leftover
> accessId row is replicated to every OM.
> * Any OM that restarts, or installs a Ratis snapshot, fails to start with
> multi-tenancy enabled. OMs that are already running keep working until then.
> * There is no repair command for the tenant tables. Disabling
> {{ozone.om.multitenancy.enabled}} lets the OM start (code reading, not run),
> without tenant functionality.
> * Affects releases that contain HDDS-13361 (2.1.0 onwards). Before it the
> check ran in the apply step and rejected the delete.
> h3. Reproduction
> PASS on unmodified source.
> [^TestBugMC2TenantDeleteRacesUserAssignOverRpc.java]
> ({{hadoop-ozone/ozone-manager}}) runs one real OM ({{OmTestManagers}}: single
> node Ratis, real apply and double buffer) through the public {{ObjectStore}}
> API, with the in memory access controller used by
> {{ozone.om.tenant.dev.skip.ranger}} in place of Ranger. The only controlled
> step is that the test holds the tenant volume's lock, which keeps the
> assignment's apply waiting and stands in for its commit and apply taking a
> few hundred ms. The delete is sent once the assignment is in the Raft log,
> and the volume lock is released once the delete waits for the authorizer lock
> (both by polling). Then the test calls {{OzoneManager.stop()}} and
> {{restart()}}. Output on unmodified source:
> {noformat}
> assign -> OK, delete -> OK
> tenantStateTable[tenant1] exists = false, tenantAccessIdTable[tenant1$user1]
> exists = true
> restart -> java.lang.NullPointerException: OmDBTenantState should have
> existed for tenant1
> {noformat}
> A control test assigns first, then deletes, and gets "Tenant 'tenant1' is not
> empty", after which the restart succeeds. A passing test means the defect is
> present.
> [^TestBugMC2TenantDeleteRacesUserAssign.java] is a lower level variant that
> runs {{preExecute}} on two threads and the apply in log order by hand, and
> builds {{OMMultiTenantManagerImpl}} over the reopened DB.
> h3. Suggested fix
> The patch prevents new orphan rows. A cluster that already has one still
> needs a way out, for example {{loadTenantCacheFromDB}} logging and skipping
> such a row instead of refusing to start, or an {{ozone repair om}} command.
> That changes deliberate fail fast behaviour, so it is left for a separate
> change.
> h3. Patch
> [^MC-2-check-tenant-empty-under-authorizer-lock-on-delete.patch], against
> a6b7bdb937109ba2893688b41a89470bc95c88cf. It also applies to master at
> b0aa6475b78.
> The emptiness check moves into the locked section of
> {{OMTenantDeleteRequest.preExecute}}, ahead of the Ranger delete, so a failed
> check releases the lock through the existing {{catch}}. A delete that waited
> for an assignment now sees its user and fails with {{TENANT_NOT_EMPTY}}
> before touching Ranger. Message, result code and the HDDS-13361 property that
> the check runs before any Ranger change are kept. No apply side check is
> added: a leader accepts tenant requests only when it is ready (all earlier
> entries applied), and each tenant request holds the lock until its own apply,
> so the check under the lock already sees every earlier tenant entry. An apply
> side check would also change replicated behaviour without a layout feature.
> For a delete that fails anyway, the order of checks changes: a caller without
> WRITE_ACL on the tenant volume now gets {{PERMISSION_DENIED}} before
> {{TENANT_NOT_EMPTY}}, and a delete of a non-empty tenant can get {{TIMEOUT}}
> while another tenant request holds the lock.
> Covered by {{preExecuteRejectsTenantAssignedWhileWaitingForLock}} and the
> control {{preExecuteDeletesEmptyTenantAndKeepsLock}} in the existing
> {{TestOMTenantDeleteRequest}}. Without the change the first fails because no
> exception is thrown. With it the tenant request, S3 secret, multi tenant
> manager, authorizer lock and execution flow suites (50 tests) pass, as do
> {{TestMultiTenantVolume}} and {{TestOzoneTenantShell}} on a mini cluster (11
> tests). Checkstyle is clean. With the patch the reproduction's race test
> fails at its delete assertion (the delete is rejected because the tenant is
> not empty, both rows remain and the restart succeeds), and its control still
> passes.
> Suggested priority: Major. No OM can restart or install a snapshot with
> multi-tenancy enabled once an orphan row exists, and there is no repair
> command. The trigger needs an Ozone admin to delete a tenant while a user is
> being assigned to that same tenant, within about a second, and running OMs
> keep serving until they restart.
> Found by TLA+ model checking and code review of the OM multi-tenancy and
> Ranger sync paths under HDDS-15926, on commit
> a6b7bdb937109ba2893688b41a89470bc95c88cf. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. The attached patch
> is a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
