[GitHub] [hudi] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-1002441676 > I'm not in favor of fat Enum either. But would like to understand the main benefit of this change: is it meant for portability of these logic? @RocMarshal Thanks @xushiyan for the review. After listening to your opinions, I don't think this reconstruction seems to be much significant. Or turn it off. If new strategies are introduced in the future, it goes perhaps without good scalability. -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-998502857 @vinothchandar I made some change based on your comments. PTAL. Thanks. -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-982832067 Excuse me, @xushiyan Shall we close the PR ? -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-976249155 cc @leesf -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-976191473 @hudi-bot run azure -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-971646258 @hudi-bot run azure -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-971482574 @hudi-bot run azure -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-970178412 @yanghua Thanks so much for your suggestions, which appears to have a good result on checking check the Azure CI. Thanks again. -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-968271847 @xushiyan @yanghua @leesf . Excuse me.I looked into this CI failure case in the backgroud of this pr. There are some interesting appearance in test cases , such as `TestHoodieSparkMergeOnReadTableInsertUpdateDelete`, etc. ` String newCommitTime = "001"; // ---(1) client.startCommitWithTime(newCommitTime); List records = dataGen.generateInserts(newCommitTime, 200); insertRecords(metaClient, records, client, cfg, newCommitTime); ` The new `CompactionTriggerStrategy` will use commit instant time parameters and use delta commit info parameters to judge the state. When the variable "001" tagged with `(1)` was transported into `HoodieInstantTimeGenerator#parseInstantTime(String timestamp)` , the `newCommitTime` value was not compatible with the parse pattern `MMddHHmmss` declared in `HoodieInstantTimeGenerator#INSTANT_TIME_FORMATTER`. At the same time, the compact configuration item was set as 'NUM_COMMITS' . As a result of the setting, the old `CompactionTriggerStrategy` would judge the state without instantTime parameters. The new `CompactionTriggerStrategy` will use both instantTime and commitNumber parameters to judge the state on the contrary, which is the reason why the old `CompactionTriggerStrategy` could be successful but new `CompactionTriggerStrategy` would be unsuccessful. With my limited read, we could enhance the validation of commitTime `(1)` in the target format `MMddHHmmss` or stop the refactor in the pr. Please let me know what's your opinion. -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-953446456 > @RocMarshal Please check the Azure CI. Thanks @yanghua for the review. And Thanks @nsivabalan for looking into this. I'll check the Azure CI ASAP. -- 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] RocMarshal commented on pull request #3813: [HUDI-2563][hudi-client] Refactor CompactionTriggerStrategy.
RocMarshal commented on pull request #3813: URL: https://github.com/apache/hudi/pull/3813#issuecomment-950336344 > @RocMarshal Thanks for your contribution. Can you describe if there is any test case to verify your changes? it's lready covered in org.apache.hudi.table.action.compact.TestInlineCompaction -- 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
