[GitHub] [hudi] manojpec commented on a change in pull request #3843: [HUDI-2468] Metadata table support for rolling back the first commit

2021-10-22 Thread GitBox


manojpec commented on a change in pull request #3843:
URL: https://github.com/apache/hudi/pull/3843#discussion_r734772787



##
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java
##
@@ -706,11 +707,23 @@ public HoodieEngineContext getContext() {
   }
 
   /**
-   * Fetch instance of {@link HoodieTableMetadataWriter}.
+   * Get Table metadata writer.
+   *
+   * @return instance of {@link HoodieTableMetadataWriter
+   */
+  public final Option getMetadataWriter() {

Review comment:
   Created following tickets to track related tasks
   https://issues.apache.org/jira/browse/HUDI-2603
   https://issues.apache.org/jira/browse/HUDI-2604




-- 
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-unsubscr...@hudi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [hudi] manojpec commented on a change in pull request #3843: [HUDI-2468] Metadata table support for rolling back the first commit

2021-10-22 Thread GitBox


manojpec commented on a change in pull request #3843:
URL: https://github.com/apache/hudi/pull/3843#discussion_r734702790



##
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java
##
@@ -706,11 +707,23 @@ public HoodieEngineContext getContext() {
   }
 
   /**
-   * Fetch instance of {@link HoodieTableMetadataWriter}.
+   * Get Table metadata writer.
+   *
+   * @return instance of {@link HoodieTableMetadataWriter
+   */
+  public final Option getMetadataWriter() {

Review comment:
   Except for CommitMetadata all other actions are extending 
SpecificRecordBase and are good to be moved. So, getMetadataWriter() version 
has to stay around till the CommitMetadata is solved. I can file a new ticket 
to take this on if you are ok. 




-- 
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-unsubscr...@hudi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [hudi] manojpec commented on a change in pull request #3843: [HUDI-2468] Metadata table support for rolling back the first commit

2021-10-22 Thread GitBox


manojpec commented on a change in pull request #3843:
URL: https://github.com/apache/hudi/pull/3843#discussion_r734286434



##
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java
##
@@ -233,24 +253,33 @@ public void initTableMetadata() {
 }
   }
 
-  protected void bootstrapIfNeeded(HoodieEngineContext engineContext, 
HoodieTableMetaClient dataMetaClient) throws IOException {
+  /**
+   * Bootstrap the metadata table if needed.
+   *
+   * @param engineContext  - Engine context
+   * @param dataMetaClient - Meta client for the data table
+   * @param actionMetadata - Optional action metadata
+   * @param - Action metadata types extending Avro generated 
SpecificRecordBase
+   * @throws IOException
+   */
+  protected  void 
bootstrapIfNeeded(HoodieEngineContext engineContext,
+  
HoodieTableMetaClient dataMetaClient,
+  Option 
actionMetadata) throws IOException {
 HoodieTimer timer = new HoodieTimer().startTimer();
-boolean exists = dataMetaClient.getFs().exists(new 
Path(metadataWriteConfig.getBasePath(), HoodieTableMetaClient.METAFOLDER_NAME));
+
+boolean exists = dataMetaClient.getFs().exists(new 
Path(metadataWriteConfig.getBasePath(),
+HoodieTableMetaClient.METAFOLDER_NAME));
 boolean rebootstrap = false;
+
+// If the un-synced instants have been archived, then
+// the metadata table will need to be bootstrapped again.
 if (exists) {
-  // If the un-synched instants have been archived then the metadata table 
will need to be bootstrapped again
-  HoodieTableMetaClient metadataMetaClient = 
HoodieTableMetaClient.builder().setConf(hadoopConf.get())
+  final HoodieTableMetaClient metadataMetaClient = 
HoodieTableMetaClient.builder().setConf(hadoopConf.get())
   .setBasePath(metadataWriteConfig.getBasePath()).build();
-  Option latestMetadataInstant = 
metadataMetaClient.getActiveTimeline().filterCompletedInstants().lastInstant();
-  if (!latestMetadataInstant.isPresent()) {
-LOG.warn("Metadata Table will need to be re-bootstrapped as no 
instants were found");
-rebootstrap = true;
-  } else if 
(!latestMetadataInstant.get().getTimestamp().equals(SOLO_COMMIT_TIMESTAMP)
-  && 
dataMetaClient.getActiveTimeline().getAllCommitsTimeline().isBeforeTimelineStarts(latestMetadataInstant.get().getTimestamp()))
 {
-// TODO: Revisit this logic and validate that filtering for all 
commits timeline is the right thing to do
-LOG.warn("Metadata Table will need to be re-bootstrapped as un-synced 
instants have been archived."
-+ " latestMetadataInstant=" + 
latestMetadataInstant.get().getTimestamp()
-+ ", latestDataInstant=" + 
dataMetaClient.getActiveTimeline().firstInstant().get().getTimestamp());
+  final Option latestMetadataInstant =
+  
metadataMetaClient.getActiveTimeline().filterCompletedInstants().lastInstant();
+
+  if (isBootstrapNeeded(latestMetadataInstant, actionMetadata)) {

Review comment:
   Right, fixed it. 

##
File path: 
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java
##
@@ -110,15 +124,20 @@ protected HoodieBackedTableMetadataWriter(Configuration 
hadoopConf, HoodieWriteC
   enabled = true;
 
   // Inline compaction and auto clean is required as we dont expose this 
table outside
-  ValidationUtils.checkArgument(!this.metadataWriteConfig.isAutoClean(), 
"Cleaning is controlled internally for Metadata table.");
-  
ValidationUtils.checkArgument(!this.metadataWriteConfig.inlineCompactionEnabled(),
 "Compaction is controlled internally for metadata table.");
+  ValidationUtils.checkArgument(!this.metadataWriteConfig.isAutoClean(),
+  "Cleaning is controlled internally for Metadata table.");
+  
ValidationUtils.checkArgument(!this.metadataWriteConfig.inlineCompactionEnabled(),
+  "Compaction is controlled internally for metadata table.");
   // Metadata Table cannot have metadata listing turned on. (infinite 
loop, much?)
-  
ValidationUtils.checkArgument(this.metadataWriteConfig.shouldAutoCommit(), 
"Auto commit is required for Metadata Table");
-  
ValidationUtils.checkArgument(!this.metadataWriteConfig.isMetadataTableEnabled(),
 "File listing cannot be used for Metadata Table");
+  
ValidationUtils.checkArgument(this.metadataWriteConfig.shouldAutoCommit(),

Review comment:
   As suggested in the wiki I have the reformatting for changed lines (line 
140). IDE for some reasons chose to format the above lines closer it also. 
Anyways these lines were crossing the length of 120 and I broke the log 
statements into multi line for easy readability and not to cross 120 width. 

##
File path: