[
https://issues.apache.org/jira/browse/HDDS-16825?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16825:
------------------------------
Attachment: CR-4-lease-hard-limit-counts-hsync.patch
> OM closes the file of a live hsync writer at the lease hard limit and drops
> data that hsync acknowledged
> --------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16825
> URL: https://issues.apache.org/jira/browse/HDDS-16825
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments: CR-4-lease-hard-limit-counts-hsync.patch,
> TestBugCR4LeaseLimitsIgnoreHsyncInSameBlock.java
>
>
> h3. Mechanism
> {{OmMetadataManagerImpl.getExpiredOpenKeys}} treats an hsynced open key as
> expired when the modification time of the open key is older than
> {{ozone.om.lease.hard.limit}} (default 7d). {{OpenKeyCleanupService}} then
> commits the file with the block list and length of the committed entry
> ({{fileTable}} or {{keyTable}}), which are those of the last hsync that
> reached OM. The datanodes are not asked.
> hsync does not move that time:
> * On the OM, an hsync commit updates the committed entry. It writes the open
> key only for the first hsync of a key
> ({{OmKeyHSyncUtil.isHSyncedPreviously}}). After that the modification time of
> the open key moves only when a block is allocated.
> * On the client, {{BlockOutputStreamEntryPool.hsyncKey}} (HDDS-9387) and
> {{BlockDataStreamOutputEntryPool.hsyncKey}} (HDDS-11816) call OM only for the
> first hsync of each block. Later hsync calls in the block go to the datanodes
> only.
> So a writer that stays in one block looks idle to OM from the moment it
> entered the block, and the length OM holds for that block is that of the
> first hsync in it. This does not read as a regression: the hard limit has
> compared the modification time of the open key since it was added
> (HDDS-10141), and hsync has never refreshed it.
> h3. Trigger
> A writer hsyncs a file and stays in the same block for longer than
> {{ozone.om.lease.hard.limit}}. No second client and no failure is needed.
> With the defaults (7d, 256MB blocks) that is a file that is kept open for
> more than 7 days and grows by less than one block in that time, which is
> below about 440 bytes per second on average. A cluster with a shorter hard
> limit reaches it sooner.
> h3. Impact
> OM closes the file under a live writer at a length below what hsync
> acknowledged: everything the writer hsynced in its current block after the
> first hsync there is missing from the closed file. The writer is not told.
> Its hsync calls keep returning successfully before and after the commit,
> because they do not reach OM. In the reproduction its first error comes when
> it closes the stream.
> HDDS-10141 describes the limit as the lifetime of the open file ("if file
> lifetime is already beyond" the limit) and accepts that the automatic commit
> "might lose some data at the block end which is hsynced". The code measures
> something else, the time since the last block allocation (or the first hsync
> of the key), so a writer that fills blocks is never expired and one that
> writes slowly is. Whichever definition is meant, closing a file while the
> hsync calls of its writer are being acknowledged, at a length below those
> acknowledgements and without an error to the writer, is not what hsync
> promises.
> h3. Reproduction
> PASS on unmodified source, nothing injected.
> [^TestBugCR4LeaseLimitsIgnoreHsyncInSameBlock.java]
> ({{hadoop-ozone/integration-test}}) starts a mini cluster with a hard limit
> of 10s and an open key cleanup interval of 500ms. In
> {{liveWriterIsAutoCommittedAtLengthOfFirstHsync}} the writer writes 100 bytes
> and calls hsync every 300ms for 15s. Every hsync succeeds, the file is then
> closed at OM with a length of 100 bytes, and the close of the writer fails. A
> passing test means the defect is present. The second test of the class
> belongs to the soft limit section below.
> h3. Suggested fix
> The attached patch makes the expiry follow hsync without writing anything new
> when a request is applied:
> * {{getExpiredOpenKeys}} expires an hsynced key only if the committed entry,
> which every hsync commit already updates, is older than the hard limit as
> well. The scan runs on the leader only and submits an ordinary commit, so the
> decision is still made in one place.
> * The client sends an hsync to OM again when its last update of the key at OM
> is older than a new client setting, {{ozone.client.hsync.om.update.interval}}
> (default 1h, with 0 every hsync goes to OM). It has to be shorter than the
> hard limit configured at OM, which the client is not told, hence a setting
> and not a fixed value.
> Points for review:
> * One hsync per interval and stream now calls OM synchronously, as the first
> hsync of a block always did. Without the change a writer inside a block does
> not need OM until the block is full. With it, an OM outage or failover at
> that moment blocks that hsync until OM answers or the failover retries of the
> client are used up (from the code, not run).
> * Clients without the change are not protected. They update OM once per block
> as before.
> * Rename, ACL changes and {{setTimes}} also update the committed entry.
> Because both times have to be older than the limit, they can only postpone
> the automatic commit, {{setTimes}} with a time in the future until that time
> or until the next hsync that reaches OM. Explicit lease recovery is not
> affected.
> Not covered:
> * The automatic commit still uses the length OM saw last. For a writer that
> died, data acknowledged by hsync calls after its last update of OM is dropped
> at the hard limit (run: 200 bytes acknowledged by two hsync calls in one
> block, file closed with 100). Without the patch that is everything after the
> first hsync in the block, with it at most one interval.
> * The soft limit, see below.
> h3. Patch
> [^CR-4-lease-hard-limit-counts-hsync.patch], against
> ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199. It also applies to master at
> 84f5594a6a2.
> Covered by {{testHsyncWithinHardLimitKeepsKeyOpen}} in the existing
> {{TestOpenKeyCleanupService}}, which fails without the change in
> {{getExpiredOpenKeys}}, and by one more case in
> {{TestHSync.testHsyncKeyCallCount}}, which fails without the client change.
> The description of {{ozone.om.lease.hard.limit}} in {{ozone-default.xml}} is
> updated. With the patch {{TestOpenKeyCleanupService}},
> {{TestOmMetadataManager}}, the key commit and recover lease request suites of
> OM (98 tests together), {{TestHSync}} (42 tests) and the client unit suites
> pass, and checkstyle is clean. With the patch the first test of the
> reproduction fails, because the file stays open while its writer keeps
> hsyncing.
> h3. Soft limit: an observation and a question
> {{OMRecoverLeaseRequest}} rejects {{recoverLease}} without force with
> {{KEY_UNDER_LEASE_SOFT_LIMIT_PERIOD}} while the modification time of the open
> key is younger than {{ozone.om.lease.soft.limit}} (default 60s). That is the
> same time as above, so the check measures the time since the last block
> allocation of the writer, not since its last hsync. Recovery without force
> therefore succeeds against a live writer once it has stayed in one block for
> the soft limit. In the second test of the reproduction, with a limit of 3s,
> it is rejected right after the first hsync and accepted 4s later,
> milliseconds after another successful hsync. Recovery asks the datanodes for
> the length there, so nothing that was hsynced is lost.
> Whether that is wanted is a question, not a defect claim. The review of the
> pull request that added the limit (#5974, HDDS-10104) gives its purpose as
> keeping a recoverer from taking a file too soon while a client is still
> writing to it, which it does not do for a writer that stays in one block. On
> the other side, HBase WAL splitting calls {{recoverLease}} on the log of a
> region server that may still be running and retries it (HDDS-10651), and HDFS
> {{recoverLease}} releases the lease without waiting for its soft limit.
> Making hsync count for the soft limit would turn a writer that can be fenced
> into one that cannot be fenced without the force flag, so this patch
> deliberately leaves the check as it is. A client side lease renewal was
> started in HDDS-8600 and not finished. Related: HDDS-16767 (same check,
> different defect).
> Found by code review of the lease recovery paths for hsynced files, as part
> of the TLA+ verification effort under HDDS-15926, on commit
> ea69b4a7d9abd040e5259dbcbb7c6ce52eb5d199. 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]
