[
https://issues.apache.org/jira/browse/HDDS-17178?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17178:
------------------------------
Attachment:
CR-1-unregister-secret-manager-certificate-receiver-on-stop.patch
> Stopped OM delegation token secret manager stays registered with the
> certificate client, so a checkpoint install leaks the replaced manager
> -------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17178
> URL: https://issues.apache.org/jira/browse/HDDS-17178
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> CR-1-unregister-secret-manager-certificate-receiver-on-stop.patch,
> TestBugCR1StaleSecretManagerReceivers.java
>
>
> h3. Mechanism
> {{OzoneSecretManager.start}} registers the manager with
> {{CertificateClient.registerNotificationReceiver}}, and
> {{OzoneSecretManager.stop}} only clears the running flag.
> {{CertificateClient}} has no way to remove a receiver, and
> {{DefaultCertificateClient}} keeps its receivers in a {{HashSet}} for the
> life of the process.
> When a follower OM installs a Ratis checkpoint,
> {{OzoneManager.installCheckpoint}} stops the delegation token secret manager
> and the metadata manager, and {{reloadOMState}} calls
> {{instantiateServices}}, which builds a new {{OmMetadataManagerImpl}} and a
> new {{OzoneDelegationTokenSecretManager}} and then starts the new manager
> with the same {{certClient}}. The replaced manager stays in the receiver set.
> Through its {{OzoneSecretStore}} it keeps the closed
> {{OmMetadataManagerImpl}} reachable, including the full table caches of the
> volume, bucket and revoked STS token tables, and it keeps its own token map.
> {{OmMetadataManagerImpl.stop}} closes the RocksDB instance, so what is
> retained is heap.
> h3. Trigger
> # Security is enabled and an OM installs a checkpoint from the leader (it
> fell behind the leader's purged log, or it is being bootstrapped).
> # Each install that stops the old metadata manager and runs {{reloadOMState}}
> adds one stopped secret manager and one closed metadata manager to the
> certificate client's receiver set, until the OM process restarts. An install
> that does not stop the metadata manager (for example when the checkpoint is
> not ahead of this OM) restarts the same secret manager and leaks nothing.
> # Every later OM certificate renewal calls {{notifyCertificateRenewed}} on
> each of them. They only update their own key and log "Certificate is
> changed", so tokens are not affected.
> h3. Impact
> Per install, roughly one copy of the volume and bucket caches plus the
> delegation tokens known at that time stays on the heap until the OM restarts
> (the replaced manager also keeps the old {{S3SecretManager}} and the closed
> metadata manager's {{SnapshotChainManager}}). Checkpoint installs are rare in
> a healthy cluster, so the growth is slow; it adds up for an OM that installs
> checkpoints repeatedly during a long uptime, and for clusters with many
> buckets. No effect on token correctness was found. The registration was added
> by HDDS-7723, so releases from 1.4.0 on are affected.
> Related: HDDS-16853 is the same pattern for the multi tenant manager. With
> multi tenancy enabled, its leaked Ranger sync service also keeps the old
> metadata manager reachable, so this patch alone does not release that heap
> there; both fixes are needed.
> h3. Reproduction
> PASS on the unmodified source. [^TestBugCR1StaleSecretManagerReceivers.java]
> ({{hadoop-ozone/ozone-manager}}) uses a real {{OMCertificateClient}} (its
> key, certificate and cert path getters stubbed; the receiver set and
> notification path are real) and real {{OmMetadataManagerImpl}} instances on
> one DB holding 1000 buckets, and replays the secret manager calls of
> {{installCheckpoint}} and {{reloadOMState}} three times. It does not run
> {{installCheckpoint}} itself in a secure mini cluster. Output on the
> unmodified source:
> {noformat}
> replaced secret managers still reachable: 3/3
> replaced (stopped) metadata managers still reachable: 3/3
> stopped metadata manager bucket cache entries: 1000
> stopped metadata manager bucket cache entries: 1000
> stopped metadata manager bucket cache entries: 1000
> stopped secret managers that took the renewed certificate: 3/3
> {noformat}
> A secret manager and metadata manager built the same way but never started
> are garbage collected in the same run, which shows that the registration is
> what keeps the others alive. A passing test means the defect is present. With
> the patch the replaced managers are collected and not notified, so the test
> fails.
> h3. Patch
> [^CR-1-unregister-secret-manager-certificate-receiver-on-stop.patch], against
> b94df213f20ec0d675f079b57a4566e4e64e4060. It also applies to master at
> df42f7f60a2.
> Adds {{CertificateClient.unregisterNotificationReceiver}} (implemented in
> {{DefaultCertificateClient}} and in the test {{CertificateClientTestImpl}})
> and calls it from {{OzoneSecretManager.stop}}. A manager that is stopped and
> started again (the install path that does not stop the metadata manager, for
> example when the checkpoint is not ahead of this OM) registers itself again
> in {{start}}. {{OzoneDelegationTokenSecretManager}} is the only subclass of
> {{OzoneSecretManager}}; the SSL key and trust managers, the only other
> receivers, are created once per certificate client and are not affected.
> Covered by {{testStoppedSecretManagerIsNotNotifiedOfCertificateRenewal}} in
> the existing {{TestOzoneDelegationTokenSecretManager}}, which also checks
> that a restarted manager is still notified. Without the change it fails
> because the replaced manager takes the renewed certificate ({{Expecting
> AtomicInteger(2) to have value: 1}}). With it
> {{TestOzoneDelegationTokenSecretManager}} (17 tests),
> {{TestDefaultCertificateClient}}, {{TestDnCertificateClientInit}},
> {{TestRootCertificate}} and {{TestSecretKeyManager}} (33 tests) and
> {{TestSecureOzoneCluster}} on a mini cluster (9 tests) pass, and checkstyle
> is clean. With the patch the reproduction reports 0 of 3 replaced managers
> and metadata managers still reachable and 0 of 3 notified.
> Found by code review of the OM delegation token secret manager and request
> paths, as part of the TLA+ verification effort under HDDS-15926, on commit
> b94df213f20ec0d675f079b57a4566e4e64e4060. 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]
