hudi-agent commented on code in PR #19794:
URL: https://github.com/apache/hudi/pull/19794#discussion_r3900936164
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/FileSystemBasedLockProvider.java:
##########
@@ -293,9 +293,12 @@ public static TypedProperties getLockConfig(String
tablePath) {
/**
* Returns the default lock file root path.
*
- * <p>IMPORTANT: this path should be shared especially when there is engine
cooperation.
+ * <p>IMPORTANT: this path should be shared especially when there is engine
cooperation. It lives
+ * under the table metadata folder ({@code .hoodie}) so every engine/task
derives the same lock
+ * file location from the table base path. This is also the fallback used by
the constructor when
+ * no explicit lock path is configured, so the two must stay in sync.
*/
private static String defaultLockPath(String tablePath) {
- return tablePath + StoragePath.SEPARATOR + AUXILIARYFOLDER_NAME;
+ return tablePath + StoragePath.SEPARATOR +
HoodieTableMetaClient.METAFOLDER_NAME;
Review Comment:
🤖 Changing `defaultLockPath` from `.aux` to `.hoodie` also moves the lock
location for the other `getLockConfig()` callers — `hudi-flink`
(`StreamerUtil.getLockConfig`) and `hudi-utilities` (`UtilHelpers`), which
previously resolved to `.aux`. During a rolling upgrade, an old-version writer
holding `.aux/lock` and a new-version writer holding `.hoodie/lock` would no
longer mutually exclude and could update the MDT concurrently. Have you
considered the mixed-version window here, and is a migration note warranted?
@nsivabalan could you weigh in on whether this cross-engine lock-path move
needs a compatibility step?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark-common/src/main/scala/org/apache/hudi/HoodieCLIUtils.scala:
##########
@@ -60,13 +59,26 @@ object HoodieCLIUtils extends Logging {
}
// Priority: defaults < catalog props < table config < sparkSession conf <
specified conf
- val finalParameters = HoodieWriterUtils.parametersWithWriteDefaults(
+ val parameters = HoodieWriterUtils.parametersWithWriteDefaults(
(catalogProps ++
metaClient.getTableConfig.getProps.asScala.toMap ++
filterHoodieConfigs(sparkSession.sqlContext.getAllConfs) ++
conf).toMap
Review Comment:
🤖 nit: the 5-line comment mostly restates what the surrounding code already
shows — could you trim it to just the non-obvious constraint, something like
`// Must be applied before building the client; see getLockOptions for why FS
atomic-creation support is required.`?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
