ad1happy2go commented on code in PR #20063:
URL: https://github.com/apache/hudi/pull/20063#discussion_r4110693265
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -422,13 +422,21 @@ private void
performRollbackAndCompactionIfRequired(HoodieTableVersion fromVersi
* does not carry {@link HoodieTableConfig#COMPLEX_KEYGEN_ENCODING} yet, and
persists it with the version change.
* The data has to be read up-front: the 7 to 8 hop rewrites the timeline on
storage while
* {@code hoodie.properties} still reports the old version.
+ *
+ * <p>The data is read through a meta client that takes the timeline layout
from {@code hoodie.properties}:
+ * callers such as the upgrade procedure and hudi-cli pin the layout of the
write config, which is newer than
+ * the layout a table below version 8 has on storage, so their meta client
cannot load its timeline.
*/
private void resolveComplexKeygenEncoding(Map<ConfigProperty, String>
tablePropsToAdd, String operation) {
HoodieTableConfig tableConfig = metaClient.getTableConfig();
if (!KeyGenUtils.requireComplexKeyGenEncodingTracked(tableConfig) ||
tableConfig.getComplexKeyGenEncoding().isPresent()) {
return;
}
- ComplexKeyGenEncoding encoding =
KeyGenUtils.resolveComplexKeyGenEncodingForWrite(metaClient, config)
+ HoodieTableMetaClient dataMetaClient = HoodieTableMetaClient.builder()
Review Comment:
Agreed, done in 443d0bb7. The second meta client in `UpgradeDowngrade` is
reverted; `UpgradeOrDowngradeProcedure` and `SparkMain` no longer call
`setLayoutVersion`. I checked `run()` before dropping it: the caller's meta
client is only used for the table config, storage/base path and table type, and
every hop builds its own table through `upgradeDowngradeHelper.getTable(config,
context)`, so v8+ tables are unaffected. Those two are also the only callers
that hand `UpgradeDowngrade` a pinned meta client (the write clients use the
table's own).
I only removed the pin line rather than switching to
`BaseProcedure.createMetaClient`, so the consistency-guard and retry configs
these callers pass are kept. Happy to switch if you prefer.
The regression test now goes through `call upgrade_table` in
`TestUpgradeOrDowngradeProcedure` on both v6 fixtures (`field:value` and bare
keys): it asserts the table reaches the current version with `FIELD_PREFIXED` /
`VALUE_ONLY` recorded, and that upserting the fixture records afterwards
updates all of them in place under the keys they are stored with (same key set,
no duplicates). The `TestUpgradeDowngrade` test from the first revision is
removed. With the pin put back, the new test fails (`upgrade_table` returns
false), and the upgrade/downgrade matrix I run end to end on a bundle from this
revision (genuine 0.14.0/0.14.1/1.0.2 tables, v9 tables written by 1.1.1,
downgrades) passes 13/13.
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/upgrade/UpgradeDowngrade.java:
##########
@@ -422,13 +422,21 @@ private void
performRollbackAndCompactionIfRequired(HoodieTableVersion fromVersi
* does not carry {@link HoodieTableConfig#COMPLEX_KEYGEN_ENCODING} yet, and
persists it with the version change.
* The data has to be read up-front: the 7 to 8 hop rewrites the timeline on
storage while
* {@code hoodie.properties} still reports the old version.
+ *
+ * <p>The data is read through a meta client that takes the timeline layout
from {@code hoodie.properties}:
+ * callers such as the upgrade procedure and hudi-cli pin the layout of the
write config, which is newer than
+ * the layout a table below version 8 has on storage, so their meta client
cannot load its timeline.
*/
private void resolveComplexKeygenEncoding(Map<ConfigProperty, String>
tablePropsToAdd, String operation) {
HoodieTableConfig tableConfig = metaClient.getTableConfig();
if (!KeyGenUtils.requireComplexKeyGenEncodingTracked(tableConfig) ||
tableConfig.getComplexKeyGenEncoding().isPresent()) {
return;
}
- ComplexKeyGenEncoding encoding =
KeyGenUtils.resolveComplexKeyGenEncodingForWrite(metaClient, config)
+ HoodieTableMetaClient dataMetaClient = HoodieTableMetaClient.builder()
+ .setStorage(metaClient.getStorage())
+ .setBasePath(metaClient.getBasePath())
+ .build();
+ ComplexKeyGenEncoding encoding =
KeyGenUtils.resolveComplexKeyGenEncodingForWrite(dataMetaClient, config)
Review Comment:
Makes sense, added in 80b7690f as a separate commit.
`deduceComplexKeyGenEncodingFromData` now returns empty ("cannot deduce") when
the version 1 timeline has no completed commit but the table is below version 8
and already has the version 2 timeline directory, which only an unfinished 7 to
8 hop creates. With validation on (the default), the upgrade/write then fails
with the usual complex keygen message instead of recording `FIELD_PREFIXED`;
with validation off, the configured encoding applies as before. Covered by
`TestKeyGenUtils.testComplexKeyGenEncodingNotDeducedFromTimelineMovedByUnfinishedUpgrade`,
which fails without the new branch. Since the write path shares the deduction,
it gets the same protection.
--
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]
