Siyao Meng created HDDS-16856:
---------------------------------
Summary: 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
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]
