HIVE-28822: Concurrent INSERTs can silently lose rows or fail with FileAlreadyExistsException on S3 (non-ACID) - #6642
Conversation
|
@abstractdog recent fix in this area: 5ae5a70 cc @difin |
yeah, I confirmed that it didn't solve the problem I was investigating completely so that's why I would really appreciate a review on this patch from you guys :) |
c4ab838 to
977579c
Compare
asf-ci-hive
added
tests pending
tests unstable
and removed
tests unstable
tests pending
labels
4f81258 to
71f6fe8
Compare
none of the issues was introduced by this patch, this also fixed brain method problem by refactoring logic to a new method |
| * the set of unsafe filesystems is a property of the filesystem implementation, not something an | ||
| * operator should override. | ||
| */ | ||
| public enum UnstableRenameFileSystem { |
There was a problem hiding this comment.
Anything named *FileSystem in this codebase is a FileSystem subclass.
Why not simply use set
private static final Set<String> NON_ATOMIC_RENAME_SCHEMES = ImmutableSet.of("s3a", "s3n", "s3", "gs");
maybe introduce instead allowlist - the known-safe (hdfs, file, viewfs) ?
There was a problem hiding this comment.
ack, and this is over-engineered too, fixed in aab3c3b
| S3("s3"), | ||
| // Google Cloud Storage exposes the same "rename is copy+delete" semantics through the Hadoop | ||
| // connector; keep here so multi-cloud deployments are covered without further edits. | ||
| GS("gs"); |
There was a problem hiding this comment.
did you consider abfs? please check BlobStorageUtils
There was a problem hiding this comment.
cannot make sure, depends on whether hierarchical namespaces are enabled
The atomic rename feature is not supported by the ABFS scheme ; however, rename, create and delete operations are atomic if Namespace is enabled for your Azure Storage account.
https://hadoop.apache.org/docs/stable/hadoop-azure/abfs.html#Rename_Options
https://learn.microsoft.com/en-us/azure/storage/blobs/data-lake-storage-namespace
given the new implementation has no performance implications, I'm simply applying this for abfs too, instead of hacking further to decide whether it's hiearchical or not
| if (qid == null || qid.isEmpty()) { | ||
| return ""; | ||
| } | ||
| return String.format("%08x", qid.hashCode()); |
There was a problem hiding this comment.
HIVE_QUERY_ID = hive_<ts>_<uuid>, can we extract uuid from there?
There was a problem hiding this comment.
maybe move to QueryPlan next to makeQueryId
public static String extractUniquenessTag(String queryId) {
UUID uuid = UUID.fromString(queryId.substring(queryId.lastIndexOf('_') + 1));
return String.format("%016x", uuid.getMostSignificantBits());
}
| */ | ||
| static String computeUniquenessTag(HiveConf conf) { | ||
| String qid = HiveConf.getVar(conf, ConfVars.HIVE_QUERY_ID); | ||
| if (qid == null || qid.isEmpty()) { |
There was a problem hiding this comment.
if (StringUtils.isEmpty(qid)) {
throw new IllegalStateException("hive.query.id is required to derive a unique destination name");
}
|
do we need to fix Utilities.moveFile as well ? |
the same pattern, yes, created follow-up ticket about that: https://issues.apache.org/jira/browse/HIVE-29775 |
aab3c3b to
7a10a00
Compare
|
The tag is per-query, but a single query can move multiple files with the same basename into the same destination directory, isn't it? FS On master, the exists-probe loop resolves it (000000_0 + 000000_0_copy_1); under the PR both legs compute the same 000000_0_copy_ and collide HIVE-21100 seems to add branch index, so we might be sorted |
| final String fullName = sourcePath.getName(); | ||
|
|
||
| final String name; | ||
| if (taskId == -1) { // non-acid |
There was a problem hiding this comment.
are you sure it also covers Insert-only (MM) transactional tables? otherwise we might fail in AcidUtils
There was a problem hiding this comment.
micromanaged tables were tested, they never hit this codepath, and properly ends up in the same folder structure as they are already separated by the delta dirs, here is the manual testing:
mvn clean install -Dtest=StartMiniHS2Cluster -DminiHS2.clusterType=llap -DminiHS2.run=true -DminiHS2.usePortsFromConf=true -T 1C -Denforcer.skip=true -pl itests/hive-unit -Pitests -nsu -DminiHS2.isMetastoreRemote=true
set hive.support.concurrency=true;
set hive.exec.dynamic.partition.mode=nonstrict;
set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
create table p_test (a int) partitioned by (b int)
stored as parquet
location 's3a://dw-team-bucket/tmp/p_test'
tblproperties (
'transactional' = 'true',
'transactional_properties' = 'insert_only'
);
| Table Type: | MANAGED_TABLE | NULL |
| | transactional_properties | insert_only |
set tez.local.mode=true;
set tez.runtime.optimize.local.fetch=true;
insert into p_test values (1, 2); <--- 30 concurrent runners
was leading to files like:
aws s3 ls --recursive s3://dw-team-bucket/tmp/p_test/b=2/
2026-07-31 09:26:13 0 tmp/p_test/b=2/
2026-07-31 09:26:17 0 tmp/p_test/b=2/delta_0000001_0000001_0000/
2026-07-31 09:26:17 417 tmp/p_test/b=2/delta_0000001_0000001_0000/000000_0
2026-07-31 09:26:17 0 tmp/p_test/b=2/delta_0000002_0000002_0000/
2026-07-31 09:26:18 417 tmp/p_test/b=2/delta_0000002_0000002_0000/000000_0
2026-07-31 09:26:14 0 tmp/p_test/b=2/delta_0000003_0000003_0000/
2026-07-31 09:26:15 417 tmp/p_test/b=2/delta_0000003_0000003_0000/000000_0
2026-07-31 09:26:16 0 tmp/p_test/b=2/delta_0000004_0000004_0000/
2026-07-31 09:26:17 417 tmp/p_test/b=2/delta_0000004_0000004_0000/000000_0
2026-07-31 09:26:16 0 tmp/p_test/b=2/delta_0000005_0000005_0000/
2026-07-31 09:26:17 417 tmp/p_test/b=2/delta_0000005_0000005_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000006_0000006_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000006_0000006_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000007_0000007_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000007_0000007_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000008_0000008_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000008_0000008_0000/000000_0
2026-07-31 09:26:18 0 tmp/p_test/b=2/delta_0000009_0000009_0000/
2026-07-31 09:26:18 417 tmp/p_test/b=2/delta_0000009_0000009_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000010_0000010_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000010_0000010_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000011_0000011_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000011_0000011_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000012_0000012_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000012_0000012_0000/000000_0
2026-07-31 09:26:18 0 tmp/p_test/b=2/delta_0000013_0000013_0000/
2026-07-31 09:26:18 417 tmp/p_test/b=2/delta_0000013_0000013_0000/000000_0
2026-07-31 09:26:19 0 tmp/p_test/b=2/delta_0000014_0000014_0000/
2026-07-31 09:26:20 416 tmp/p_test/b=2/delta_0000014_0000014_0000/000000_0
2026-07-31 09:26:17 0 tmp/p_test/b=2/delta_0000015_0000015_0000/
2026-07-31 09:26:18 417 tmp/p_test/b=2/delta_0000015_0000015_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000016_0000016_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000016_0000016_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000017_0000017_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000017_0000017_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000018_0000018_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000018_0000018_0000/000000_0
2026-07-31 09:26:18 0 tmp/p_test/b=2/delta_0000019_0000019_0000/
2026-07-31 09:26:19 417 tmp/p_test/b=2/delta_0000019_0000019_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000020_0000020_0000/
2026-07-31 09:26:22 417 tmp/p_test/b=2/delta_0000020_0000020_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000021_0000021_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000021_0000021_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000022_0000022_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000022_0000022_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000023_0000023_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000023_0000023_0000/000000_0
2026-07-31 09:26:19 0 tmp/p_test/b=2/delta_0000024_0000024_0000/
2026-07-31 09:26:20 417 tmp/p_test/b=2/delta_0000024_0000024_0000/000000_0
2026-07-31 09:26:20 0 tmp/p_test/b=2/delta_0000025_0000025_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000025_0000025_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000026_0000026_0000/
2026-07-31 09:26:22 417 tmp/p_test/b=2/delta_0000026_0000026_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000027_0000027_0000/
2026-07-31 09:26:22 417 tmp/p_test/b=2/delta_0000027_0000027_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000028_0000028_0000/
2026-07-31 09:26:22 417 tmp/p_test/b=2/delta_0000028_0000028_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000029_0000029_0000/
2026-07-31 09:26:21 416 tmp/p_test/b=2/delta_0000029_0000029_0000/000000_0
2026-07-31 09:26:21 0 tmp/p_test/b=2/delta_0000030_0000030_0000/
2026-07-31 09:26:21 417 tmp/p_test/b=2/delta_0000030_0000030_0000/000000_0
the very same is checked via a newly introduced unit test TestInsertCopySuffixOnFakeS3.javatestInsertIntoMicromanagedOnFakeS3LandsUnderDeltaSubdir
| "(?:_copy_([0-9]{1,6}))?" + // copy file index | ||
| "(\\d+)" + // taskId | ||
| "(?:_(\\d{1,6}))?" + // _<attemptId> (limited to 6 digits) | ||
| "(?:_copy_(\\d{1,6}|[\\da-fA-F]{8}))?" + // copy suffix: numeric counter, or 8-hex uniqueness tag |
There was a problem hiding this comment.
won't we fail on convertion non-ACID managed table to ACID ? AcidUtils.ORIGINAL_PATTERN_COPY won't match
There was a problem hiding this comment.
good catch, need to check
There was a problem hiding this comment.
valid concern, it was addressed by changing the pattern, also introduced unit tests that failed without properly patching this, I saw 2 different exceptions:
ERROR : DDLTask failed, DDL Operation: class org.apache.hadoop.hive.ql.ddl.table.misc.properties.AlterTableSetPropertiesOperation
org.apache.hadoop.hive.ql.metadata.HiveException: Unable to alter table. java.lang.IllegalStateException: Unexpected data file name format. Cannot convert default.t_acid_demo to transactional table. File: s3a://dw-team-bucket/tmp/t_acid_demo/000000_0_copy_f0796c02aef8435d
at org.apache.hadoop.hive.ql.metadata.Hive.alterTable(Hive.java:1007)
at org.apache.hadoop.hive.ql.metadata.Hive.alterTable(Hive.java:943)
at org.apache.hadoop.hive.ql.ddl.table.AbstractAlterTableOperation.finalizeAlterTableWithWriteIdOp(AbstractAlterTableOperation.java:163)
at org.apache.hadoop.hive.ql.ddl.table.AbstractAlterTableOperation.execute(AbstractAlterTableOperation.java:82)
at org.apache.hadoop.hive.ql.ddl.DDLTask.execute(DDLTask.java:84)
at org.apache.hadoop.hive.ql.exec.Task.executeTask(Task.java:214)
at org.apache.hadoop.hive.ql.exec.TaskRunner.runSequential(TaskRunner.java:105)
at org.apache.hadoop.hive.ql.Executor.launchTask(Executor.java:354)
at org.apache.hadoop.hive.ql.Executor.launchTasks(Executor.java:327)
at org.apache.hadoop.hive.ql.Executor.runTasks(Executor.java:244)
at org.apache.hadoop.hive.ql.Executor.execute(Executor.java:105)
at org.apache.hadoop.hive.ql.Driver.execute(Driver.java:346)
at org.apache.hadoop.hive.ql.Driver.runInternal(Driver.java:191)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:143)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:138)
at org.apache.hadoop.hive.ql.reexec.ReExecDriver.run(ReExecDriver.java:190)
at org.apache.hive.service.cli.operation.SQLOperation.runQuery(SQLOperation.java:234)
at org.apache.hive.service.cli.operation.SQLOperation$BackgroundWork$1.run(SQLOperation.java:334)
at java.base/java.security.AccessController.doPrivileged(AccessController.java:714)
at java.base/javax.security.auth.Subject.doAs(Subject.java:525)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1953)
at org.apache.hive.service.cli.operation.SQLOperation$BackgroundWork.run(SQLOperation.java:354)
at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
and:
Caused by: java.lang.IllegalArgumentException: Bucket ID out of range: -1
at org.apache.hive.com.google.common.base.Preconditions.checkArgument(Preconditions.java:134)
at org.apache.hadoop.hive.ql.io.BucketCodec$2.encode(BucketCodec.java:103)
at org.apache.hadoop.hive.ql.io.orc.VectorizedOrcAcidRowBatchReader.computeOffsetAndBucket(VectorizedOrcAcidRowBatchReader.java:797)
at org.apache.hadoop.hive.ql.io.orc.OrcInputFormat$SplitGenerator.callInternal(OrcInputFormat.java:1548)
at org.apache.hadoop.hive.ql.io.orc.OrcInputFormat$SplitGenerator$1.run(OrcInputFormat.java:1535)
at org.apache.hadoop.hive.ql.io.orc.OrcInputFormat$SplitGenerator$1.run(OrcInputFormat.java:1532)
at java.base/java.security.AccessController.doPrivileged(AccessController.java:714)
at java.base/javax.security.auth.Subject.doAs(Subject.java:525)
at org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1953)
at org.apache.hadoop.hive.ql.io.orc.OrcInputFormat$SplitGenerator.call(OrcInputFormat.java:1532)
at org.apache.hadoop.hive.ql.io.orc.OrcInputFormat$SplitGenerator.call(OrcInputFormat.java:1348)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
... 3 more
TestInsertCopySuffixOnFakeS3.java extensively tests different source tables converted to ACID, also acid_convert_16hex_copy_tag.q was added for the same
asf-ci-hive
added
tests pending
tests unstable
and removed
tests failed
tests pending
labels
ack, this has to be sorted now, because "the exists-probe loop resolves it" is just true to a certain extent, which is still subject to the reported problem, which is the race in multiple places in the copy++ loop: I'm going to address this as well and let you know regarding HIVE-21100 that's another area that might be investigated, because it claims: |
abstractdog
changed the title
7a10a00 to
5d3fe34
Compare
…tly lose rows or fail with FileAlreadyExistsException on S3 (non-ACID)
On file systems whose rename is not atomic-if-absent (S3A and other object
stores), two concurrent non-ACID INSERTs that create the same new dynamic
partition race in Hive.mvFile between the exists()-driven _copy_N picker
and the destFs.rename() call. Depending on the timing this shows up as
either:
* Fail-loud — S3AFileSystem.initiateRename throws
FileAlreadyExistsException, surfacing to the client as
"MoveTask return code 40000". This matches the customer report:
[load-dynamic-partitionsToAdd-0] Failed to move: ...
Caused by: FileAlreadyExistsException:
Failed to rename .../000001_N to .../000001_N_copy_M;
destination file exists
at S3AFileSystem.initiateRename
at Hive.mvFile
at Hive.copyFiles
at Hive.loadPartitionInternal
at Hive.lambda$loadDynamicPartitions
* Fail-silent — both writers' internal exists() probes see the target
as not-yet-present, both PUTs go to the same key, and the second
silently overwrites the first (last writer wins, no error surfaces).
Reproduced with 30 concurrent `insert into p_test values (i,2)` against
an S3-backed external Parquet table: 2 sessions fail with MoveTask, 6
rows silently missing, and the final S3 listing shows the same _copy_N
slot claimed by multiple writers.
Fix: on filesystems in UnstableRenameFileSystem (S3A/S3N/S3/GS today), the
copy suffix in Hive.mvFile carries a per-query 8-hex uniqueness tag
(derived from hive.query.id) *in place of* the numeric counter. Two
concurrent writers land at distinct destinations —
basename_copy_<queryTag1>
basename_copy_<queryTag2>
— so there is no picker loop and no rename race. On stable-rename
filesystems (HDFS, local) the historical numeric _copy_N picker is
preserved unchanged. UnstableRenameFileSystem is an in-code enum rather
than a configuration knob: the set of unsafe filesystems is a property
of the filesystem impl, not something an operator should override.
ParsedOutputFileName's copy-index regex group is widened from
`[0-9]{1,6}` to `[0-9]{1,6}|[0-9a-fA-F]{8}` so both shapes parse.
getCopyIndex returns either the numeric counter or the 8-hex tag
verbatim; downstream taskId / attemptId extraction is unaffected.
The ACID branch (taskId != -1) and the isOverwrite branch are unchanged
— ACID writers already own unique taskIds, and overwrite explicitly
clears the target first.
If a future unstable-rename filesystem is ever missed by the enum, the
failure mode is the same loud FileAlreadyExistsException →
MoveTask return code 40000 that we surface today — a correct, actionable
signal rather than a silent loss.
Verification:
* unit: TestHiveCopyFiles.testUniquenessTagAndUnstableFsGating covers
the enum recognition (matches on s3a/s3n/s3/gs, rejects hdfs/file)
and the per-query tag shape (distinct queryIds → distinct 8-hex
tags, empty queryId → empty tag). ParsedOutputFileNameTest gains
3 cases: a copy suffix that is an 8-hex tag (plain and with
extension), and a strict-shape check that rejects 7-char / non-hex
forms. All 31 tests green (20 in TestHiveCopyFiles under 4
parameterizations + 11 in ParsedOutputFileNameTest).
* end-to-end: 30-way concurrent burst against s3a://... table:
Before: 24 rows persisted, 2 MoveTask failures, many _copy_N.
After: 30 rows persisted, 0 MoveTask failures, 30 distinct
000001_N_copy_<hex> keys in S3, no FAEE.
Co-Authored-By: Claude <noreply@anthropic.com>
…nessTag; widen to 16 hex
Move the per-query uniqueness-tag helper out of Hive.mvFile's neighborhood
and into QueryPlan, next to makeQueryId() which produces the queryId shape
the tag is derived from. Widen the tag from 8 hex chars (the upper 32 bits
of the UUID's most-significant half, taken via substring) to 16 hex chars
(the full 64-bit most-significant half, taken via UUID.fromString +
getMostSignificantBits) — 16 hex is well-formed hex regardless of how
QueryPlan.makeQueryId's UUID rendering evolves, and 2^64 headroom makes
birthday-collisions vanishingly rare for any realistic per-partition
concurrency.
Layout:
* QueryPlan.extractUniquenessTag(String queryId): public static helper
that parses the UUID at the tail of the queryId and returns
String.format("%016x", uuid.getMostSignificantBits()).
* Hive.computeUniquenessTag(HiveConf): reads hive.query.id, guards
null/empty, delegates to QueryPlan.extractUniquenessTag.
* ParsedOutputFileName's copy-index regex group widens from
{[0-9a-fA-F]{8}} to {[0-9a-fA-F]{16}} so downstream filename parsing
(taskId, attemptId, copyIndex) keeps working. Numeric _copy_N form
unchanged.
* Tests updated: ParsedOutputFileNameTest exercises the 16-hex shape
and the strict-shape rejection at 15 chars / non-hex chars.
TestHiveCopyFiles.testUniquenessTagAndUnstableFsGating asserts the
exact 16-hex value produced from two known UUIDs (f47ac10b58cc4372
and 9c8a44f1e2b34a1c).
End-to-end verification: 30-way concurrent `insert into p_test values
(i,2)` against an S3-backed external table produced 30 rows, 30 distinct
000001_N_copy_<16-hex> files, zero MoveTask failures, zero FAEE.
Co-Authored-By: Claude <noreply@anthropic.com>
5d3fe34 to
5181edf
Compare
after thorough investigation it turned out this patch doesn't introduce regression in terms of union, but unions already have their own issues regardless, which are being addressed in the scope of HIVE-29798 let me describe the 2 separate union cases, which had to be checked from this patch's point of view:
destination dir: be mindful that in order to preserve this behavior, I had to add an extra
giving flattened files like: so when we hit the currently touched codepath, due to the uniqueness logic, this is the source/final path: so due to the flattening code, the files don't collide on query level due to the prefix, and they end up as final paths like below: the only problem is that these files are not ACID copy compliant due to the prefix, but it's not because of this patch and is going to be fixed in HIVE-29798: I mean, on master, a unit test already files while converting flattened files to ACID (see HIVE-29799, that I'll fixed together with HIVE-29798) |
asf-ci-hive
added
tests failed
tests pending
and removed
tests pending
tests failed
labels
|
3 participants
What changes were proposed in this pull request?
On filesystems in UnstableRenameFileSystem (S3A/S3N/S3/GS today), the copy suffix in Hive.mvFile carries a per-query 8-hex uniqueness tag (derived from hive.query.id) in place of the numeric counter. Two concurrent writers land at distinct destinations —
basename_copy_
basename_copy_
— so there is no picker loop and no rename race. On stable-rename filesystems (HDFS, local) the historical numeric _copy_N picker is preserved unchanged. UnstableRenameFileSystem is an in-code enum rather than a configuration knob: the set of unsafe filesystems is a property of the filesystem impl, not something an operator should override.
ParsedOutputFileName's copy-index regex group is widened from
[0-9]{1,6}to[0-9]{1,6}|[0-9a-fA-F]{8}so both shapes parse. getCopyIndex returns either the numeric counter or the 8-hex tag verbatim; downstream taskId / attemptId extraction is unaffected.The ACID branch (taskId != -1) and the isOverwrite branch are unchanged — ACID writers already own unique taskIds, and overwrite explicitly clears the target first.
If a future unstable-rename filesystem is ever missed by the enum, the failure mode is the same loud FileAlreadyExistsException → MoveTask return code 40000 that we surface today — a correct, actionable signal rather than a silent loss.
Why are the changes needed?
On file systems whose rename is not atomic-if-absent (S3A and other object stores), two concurrent non-ACID INSERTs that create the same new dynamic partition race in Hive.mvFile between the exists()-driven _copy_N picker and the destFs.rename() call. Depending on the timing this shows up as either:
Fail-loud — S3AFileSystem.initiateRename throws FileAlreadyExistsException, surfacing to the client as "MoveTask return code 40000". This matches the customer report:
[load-dynamic-partitionsToAdd-0] Failed to move: ...
Caused by: FileAlreadyExistsException:
Failed to rename .../000001_N to .../000001_N_copy_M;
destination file exists
at S3AFileSystem.initiateRename
at Hive.mvFile
at Hive.copyFiles
at Hive.loadPartitionInternal
at Hive.lambda$loadDynamicPartitions
Fail-silent — both writers' internal exists() probes see the target as not-yet-present, both PUTs go to the same key, and the second silently overwrites the first (last writer wins, no error surfaces).
Reproduced with 30 concurrent
insert into p_test values (i,2)against an S3-backed external Parquet table: 2 sessions fail with MoveTask, 6 rows silently missing, and the final S3 listing shows the same _copy_N slot claimed by multiple writers.Does this PR introduce any user-facing change?
It depends on whether the actual user is interested in the underlying file structure (not directory, but files).
Pre-patch, the unstable result of a highly concurrent INSERT INTO scenario was something like below: be mindful of 30 insert operations vs. 24 result files, this is just one of the symptoms I referred to as "Fail-silent" above:
After the patch, it becomes:
How was this patch tested?
unit: TestHiveCopyFiles.testUniquenessTagAndUnstableFsGating covers the enum recognition (matches on s3a/s3n/s3/gs, rejects hdfs/file) and the per-query tag shape (distinct queryIds → distinct 8-hex tags, empty queryId → empty tag). ParsedOutputFileNameTest gains 3 cases: a copy suffix that is an 8-hex tag (plain and with extension), and a strict-shape check that rejects 7-char / non-hex forms. All 31 tests green (20 in TestHiveCopyFiles under 4 parameterizations + 11 in ParsedOutputFileNameTest).
end-to-end: 30-way concurrent burst against s3a://... table: Before: 24 rows persisted, 2 MoveTask failures, many copy_N. After: 30 rows persisted, 0 MoveTask failures, 30 distinct 000001_N_copy keys in S3, no FAEE.
Disclaimer: end-to-end testing was done by Claude after I made a following small testing infra available for it:
post-patch: