You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2022/01/29 04:08:14 UTC
[GitHub] [hudi] nsivabalan commented on a change in pull request #4712: [HUDI-2809] Introduce a checksum mechanism for validating hoodie.properties
nsivabalan commented on a change in pull request #4712:
URL: https://github.com/apache/hudi/pull/4712#discussion_r794995087
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -272,15 +287,29 @@ private static void modify(FileSystem fs, Path metadataFolder, Properties modify
/// 2. delete the properties file, reads will go to the backup, until we are done.
fs.delete(cfgPath, false);
// 3. read current props, upsert and save back.
+ String checksum;
try (FSDataInputStream in = fs.open(backupCfgPath);
FSDataOutputStream out = fs.create(cfgPath, true)) {
- Properties props = new Properties();
+ Properties props = new TypedProperties();
props.load(in);
modifyFn.accept(props, modifyProps);
+ if (props.containsKey(TABLE_CHECKSUM.key()) && validateChecksum(props)) {
+ checksum = props.getProperty(TABLE_CHECKSUM.key());
+ } else {
+ checksum = String.valueOf(generateChecksum(props));
+ }
+ props.setProperty(TABLE_CHECKSUM.key(), checksum);
Review comment:
shouldn't we move this to else block only.
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -493,6 +544,13 @@ public String getUrlEncodePartitioning() {
return getString(URL_ENCODE_PARTITIONING);
}
+ /**
+ * Read the table checksum.
+ */
+ public Long getTableChecksum() {
Review comment:
why public ?
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -272,15 +287,29 @@ private static void modify(FileSystem fs, Path metadataFolder, Properties modify
/// 2. delete the properties file, reads will go to the backup, until we are done.
fs.delete(cfgPath, false);
// 3. read current props, upsert and save back.
+ String checksum;
try (FSDataInputStream in = fs.open(backupCfgPath);
FSDataOutputStream out = fs.create(cfgPath, true)) {
- Properties props = new Properties();
+ Properties props = new TypedProperties();
props.load(in);
modifyFn.accept(props, modifyProps);
+ if (props.containsKey(TABLE_CHECKSUM.key()) && validateChecksum(props)) {
+ checksum = props.getProperty(TABLE_CHECKSUM.key());
+ } else {
+ checksum = String.valueOf(generateChecksum(props));
+ }
+ props.setProperty(TABLE_CHECKSUM.key(), checksum);
props.store(out, "Updated at " + System.currentTimeMillis());
}
// 4. verify and remove backup.
// FIXME(vc): generate a hash for verification.
+ try (FSDataInputStream in = fs.open(cfgPath)) {
+ Properties props = new TypedProperties();
+ props.load(in);
+ if (!props.containsKey(TABLE_CHECKSUM.key()) || !props.getProperty(TABLE_CHECKSUM.key()).equals(checksum)) {
+ throw new HoodieIOException("Checksum property missing or does not match.");
Review comment:
is it not possible to regenerate from backup rather than failing here?
##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -272,15 +287,29 @@ private static void modify(FileSystem fs, Path metadataFolder, Properties modify
/// 2. delete the properties file, reads will go to the backup, until we are done.
fs.delete(cfgPath, false);
// 3. read current props, upsert and save back.
+ String checksum;
try (FSDataInputStream in = fs.open(backupCfgPath);
FSDataOutputStream out = fs.create(cfgPath, true)) {
- Properties props = new Properties();
+ Properties props = new TypedProperties();
props.load(in);
modifyFn.accept(props, modifyProps);
+ if (props.containsKey(TABLE_CHECKSUM.key()) && validateChecksum(props)) {
+ checksum = props.getProperty(TABLE_CHECKSUM.key());
+ } else {
+ checksum = String.valueOf(generateChecksum(props));
+ }
+ props.setProperty(TABLE_CHECKSUM.key(), checksum);
props.store(out, "Updated at " + System.currentTimeMillis());
}
// 4. verify and remove backup.
// FIXME(vc): generate a hash for verification.
+ try (FSDataInputStream in = fs.open(cfgPath)) {
Review comment:
remove L 305
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org