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