danny0405 commented on code in PR #5535:
URL: https://github.com/apache/hudi/pull/5535#discussion_r871982176
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java:
##########
@@ -1558,13 +1558,15 @@ private void tryUpgrade(HoodieTableMetaClient
metaClient, Option<String> instant
new UpgradeDowngrade(metaClient, config, context,
upgradeDowngradeHelper);
if
(upgradeDowngrade.needsUpgradeOrDowngrade(HoodieTableVersion.current())) {
- // Ensure no inflight commits by setting EAGER policy and explicitly
cleaning all failed commits
- List<String> instantsToRollback = getInstantsToRollback(metaClient,
HoodieFailedWritesCleaningPolicy.EAGER, instantTime);
+ if
(config.getWriteConcurrencyMode().supportsOptimisticConcurrencyControl()) {
+ // Ensure no inflight commits by setting EAGER policy and explicitly
cleaning all failed commits
+ List<String> instantsToRollback = getInstantsToRollback(metaClient,
HoodieFailedWritesCleaningPolicy.EAGER, instantTime);
- Map<String, Option<HoodiePendingRollbackInfo>> pendingRollbacks =
getPendingRollbackInfos(metaClient);
- instantsToRollback.forEach(entry -> pendingRollbacks.putIfAbsent(entry,
Option.empty()));
+ Map<String, Option<HoodiePendingRollbackInfo>> pendingRollbacks =
getPendingRollbackInfos(metaClient);
+ instantsToRollback.forEach(entry ->
pendingRollbacks.putIfAbsent(entry, Option.empty()));
Review Comment:
Hi @nsivabalan , i agree with your point, but at least, there is no need for
redundant metadata checking if the rollback strategy is EAGER.
And still, no one answers the question why we need the rollback (for data
set files and metadata files) before upgrade ? What is exactly the purpose here
?
@n3nash say that it is because we need to rollback the metadata table if it
is enabled, but we can not see any clues that the rollback is only for
metadata. And even if we do not rollback the metadata, the content read from
the metadata should still be correct based on that the metadata table itself is
a Hudi table and has SI visibility ?
Seems it is clear that the rollback here is not for data set files, correct
me if i'm wrong.
--
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]
