abstractdog commented on code in PR #6642:
URL: https://github.com/apache/hive/pull/6642#discussion_r3765271285
##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java:
##########
@@ -5170,6 +5189,110 @@ private static String getPathName(int taskId) {
return Utilities.replaceTaskId("000000", taskId) + "_0";
}
+ /**
+ * Compute a compact per-query uniqueness tag used by the non-ACID rename
branch of
+ * {@link #mvFile} to make each concurrent writer's destination key unique
on filesystems
+ * whose {@code rename} is not atomic-if-absent. The tag becomes the copy
suffix
+ * ({@code basename_copy_<tag>}) in place of the numeric {@code _copy_N}
counter.
+ * <p>
+ * Reads {@code hive.query.id} from the passed {@link HiveConf} and
delegates to
+ * {@link QueryPlan#extractUniquenessTag(String)} for the actual UUID → hex
derivation.
+ * The shape matches {@link ParsedOutputFileName}'s copy-index group so
downstream filename
+ * parsing (taskId, attemptId, copyIndex) keeps working.
+ */
+ static String computeUniquenessTag(HiveConf conf) {
+ String qid = HiveConf.getVar(conf, ConfVars.HIVE_QUERY_ID);
+ if (Strings.isNullOrEmpty(qid)) {
+ throw new IllegalStateException("hive.query.id is required to derive a
unique destination name");
+ }
+ return QueryPlan.extractUniquenessTag(qid);
+ }
+
+ /**
+ * @return {@code true} when the filesystem's URI scheme is one of the known
non-atomic-rename
+ * schemes ({@link #NON_ATOMIC_RENAME_SCHEMES}); {@code false}
otherwise (including a
+ * {@code null} fs or missing scheme).
+ */
+ static boolean isNonAtomicRenameFs(FileSystem fs) {
+ if (fs == null || fs.getUri() == null || fs.getUri().getScheme() == null) {
+ return false;
+ }
+ return
NON_ATOMIC_RENAME_SCHEMES.contains(fs.getUri().getScheme().toLowerCase());
+ }
+
+ /**
+ * Picks the destination {@link Path} for {@link #mvFile}, choosing between
a per-query
+ * uniqueness-tagged name (on filesystems without atomic rename-if-absent
semantics) and the
+ * legacy {@code _copy_N} counter-based picker.
+ *
+ * <p>On file systems without atomic rename-if-absent semantics (e.g. S3),
two concurrent inserts
+ * targeting the same new dynamic partition race in the counter-based picker
below: their
+ * {@code exists()} probes both fire before either PUT commits, both rename
to the same final
+ * key, and the second PUT silently overwrites the first (last writer wins,
no error surfaces).
+ * To eliminate the collision, on such filesystems we skip the counter-based
{@code _copy_N}
+ * picker entirely and use a per-query uniqueness tag (8-hex derived from
{@code hive.query.id})
+ * as the copy suffix, so two concurrent writers rename to distinct keys.
+ *
+ * <p>The uniqueness-tag path is only taken in the non-ACID rename branch
+ * ({@code taskId == -1 && isRenameAllowed && !isOverwrite}): ACID writers
already own unique
+ * taskIds, copy/copyFromLocal do not race on the destination filename, and
overwrite explicitly
+ * clears the target first.
+ */
+ private static Path pickDestFilePath(HiveConf conf, FileSystem sourceFs,
Path sourcePath, FileSystem destFs,
Review Comment:
not sure, `pickDestFilePath` is so tightly coupled to `Hive.mvFile`, they
also share almost the same huge method signature
```
private static Path mvFile(HiveConf conf, FileSystem sourceFs, Path
sourcePath, FileSystem destFs, Path destDirPath,
boolean isSrcLocal, boolean isOverwrite,
boolean isRenameAllowed,
int taskId) throws IOException {
Path destFilePath = pickDestFilePath(conf, sourceFs, sourcePath, destFs,
destDirPath, taskId, isOverwrite,
isRenameAllowed);
```
`pickDestFilePath` helps reduce the body of `mvFile`, but alone it's just a
static method, which wants to be a useful utility method, but it isn't; I
cannot see the value of refactoring it, that's why I kept it as "private
static", let me know if you want me to refactor it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
