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 2021/04/23 09:58:09 UTC

[GitHub] [hudi] MyLanPangzi opened a new pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

MyLanPangzi opened a new pull request #2868:
URL: https://github.com/apache/hudi/pull/2868


   ## *Tips*
   - *Thank you very much for contributing to Apache Hudi.*
   - *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
   
   ## What is the purpose of the pull request
   
   Remove the legacy code to avoid confusion.
   
   The list to remove:
   
   1. org.apache.hudi.sink.CommitSink
   2. org.apache.hudi.sink.InstantGenerateOperator
   3. org.apache.hudi.sink.KeyedWriteProcessFunction
   4. org.apache.hudi.sink.KeyedWriteProcessOperator
   5. org.apache.hudi.sink.transform.JsonStringToHoodieRecordMapFunction
   6. org.apache.hudi.streamer.HoodieFlinkStreamer
   7. org.apache.hudi.sink.transform.TestJsonStringToHoodieRecordMapFunction
   8. org.apache.hudi.exception.HoodieFlinkStreamerException
   
   The method to remove:
   1. StreamWriteITCase#testWriteToHoodieLegacy
   
   The list to rename:
   1. org.apache.hudi.streamer.HoodieFlinkStreamerV2 => org.apache.hudi.streamer.HoodieFlinkStreamer
   
   ## Brief change log
   
   *(for example:)*
     - *Modify AnnotationLocation checkstyle rule in checkstyle.xml*
   
   ## Verify this pull request
   
   *(Please pick either of the following options)*
   
   This pull request is a trivial rework / code cleanup without any test coverage.
   
   *(or)*
   
   This pull request is already covered by existing tests, such as *(please describe tests)*.
   
   (or)
   
   This change added tests and can be verified as follows:
   
   *(example:)*
   
     - *Added integration tests for end-to-end.*
     - *Added HoodieClientWriteTest to verify the change.*
     - *Manually verified the change by running a job locally.*
   
   ## Committer checklist
   
    - [ ] Has a corresponding JIRA in PR title & commit
    
    - [ ] Commit message is descriptive of the change
    
    - [ ] CI is green
   
    - [ ] Necessary doc changes done or have another open PR
          
    - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.


-- 
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.

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

[GitHub] [hudi] wangxianghu commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
wangxianghu commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828975932


   we can split the `hudi-utilities to
   ```
   hudi-utilities
   ├── hudi-utilities-flink
   └── hudi-utilities-spark
   ```
   Move `HoodieFlinkStreamer` to `hudi-utilities-flink` and keep kafka related dependeny to `hudi-utilities-flink-bundle`


-- 
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.

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

[GitHub] [hudi] danny0405 commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828261491


   Hi, @yanghua can you take a look, 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.

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

[GitHub] [hudi] yanghua merged pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #2868:
URL: https://github.com/apache/hudi/pull/2868


   


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828903309


   > > I still insist that we need to include kafka-related dependencies. If you look back at the HoodieFlinkStreamerV2 class. What is it in essence? It is just a program written using Flink DataStream API, which is specific (Kafka -> Hudi)
   > 
   > No, on one says that they don't know how to add a connector jar or actually few people use the `HoodieFlinkStreamerV2` tool.
   
   "one says that they don't know how to add a connector jar" -> I recommend we package it into the bundle for users. It's not that users won't, but users should not or may not need to perceive these things. This is a question of user experience.
   
   According to your logic, what reason do you think users will not use FlinkWriteClient directly? Why should we guide users to use Flink SQL? Can't users write the FlinkStreamer class by themselves?
   
   All of this is to shield users from details as much as possible, let the framework provide out-of-the-box capabilities as much as possible, and provide a good experience as much as possible? Is not it?
   
   "actually few people use the `HoodieFlinkStreamerV2` tool" -> Actually, there still few users use the flink write client, because it is still not production-ready for 0.8, you know. IMO, we do not get enough samples about your result.
   
   I have never understood why we cannot include the kafka connector to provide convenience to some users who do not use SQL. And it should provide a consistent experience with DeltaStreamer based on Spark. Otherwise, don't call "FlinkStreamerXXX".


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828976874


   > we can split the `hudi-utilities to
   > 
   > ```
   > hudi-utilities
   > ├── hudi-utilities-flink
   > └── hudi-utilities-spark
   > ```
   > 
   > Move `HoodieFlinkStreamer` to `hudi-utilities-flink` and keep kafka related dependeny to `hudi-utilities-flink-bundle`
   
   +1


-- 
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.

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

[GitHub] [hudi] MyLanPangzi commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
MyLanPangzi commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828259208


   @yanghua Hi，i triggered the ci and it's passed.


-- 
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.

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

[GitHub] [hudi] danny0405 commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828420402


   > I still insist that we need to include kafka-related dependencies. If you look back at the HoodieFlinkStreamerV2 class. What is it in essence? It is just a program written using Flink DataStream API, which is specific (Kafka -> Hudi)
   
   No, on one says that they don't know how to add a connector jar or actually few people use the `HoodieFlinkStreamerV2` tool.


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828977301


   > > > Fine, let's keep them for a time.
   > > 
   > > 
   > > I do not object to deleting the legacy code. I just suggest we'd better add the Kafka connector into the bundle. And let it be a completed function. Then, we can delete the legacy code and use the new Flink streamer.
   > 
   > I agree the Flink streamer should bundle with a kafka jar, but the hudi-flink-bundle should not include kafka, because many people include the bundle jar in the pom directly, we should keep the jar as small as possible to avoid unnecessary jar conflicts.
   > 
   > Maybe we should move the streamer out of the hudi-flink module into the hudi-utilities module.
   
   +1, it's what I mean.


-- 
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.

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

[GitHub] [hudi] codecov-commenter edited a comment on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-825580328


   # [Codecov](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2868](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3928b17) into [master](https://codecov.io/gh/apache/hudi/commit/4a3431866d995fd877a067967457591a0bc8847d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a34318) will **increase** coverage by `0.52%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hudi/pull/2868/graphs/tree.svg?width=650&height=150&src=pr&token=VTTXabwbs2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2868      +/-   ##
   ============================================
   + Coverage     53.01%   53.53%   +0.52%     
   + Complexity     3744     3732      -12     
   ============================================
     Files           488      481       -7     
     Lines         23435    23140     -295     
     Branches       2500     2468      -32     
   ============================================
   - Hits          12423    12388      -35     
   + Misses         9913     9653     -260     
     Partials       1099     1099              
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | hudicli | `39.53% <ø> (ø)` | `220.00 <ø> (ø)` | |
   | hudiclient | `∅ <ø> (∅)` | `0.00 <ø> (ø)` | |
   | hudicommon | `50.65% <ø> (ø)` | `1976.00 <ø> (ø)` | |
   | hudiflink | `63.24% <0.00%> (+4.12%)` | `524.00 <0.00> (-11.00)` | :arrow_up: |
   | hudihadoopmr | `33.33% <ø> (ø)` | `198.00 <ø> (ø)` | |
   | hudisparkdatasource | `73.33% <ø> (ø)` | `237.00 <ø> (ø)` | |
   | hudisync | `46.10% <ø> (ø)` | `142.00 <ø> (ø)` | |
   | huditimelineservice | `64.36% <ø> (ø)` | `62.00 <ø> (ø)` | |
   | hudiutilities | `69.68% <ø> (-0.06%)` | `373.00 <ø> (-1.00)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../org/apache/hudi/streamer/HoodieFlinkStreamer.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS9zdHJlYW1lci9Ib29kaWVGbGlua1N0cmVhbWVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../main/java/org/apache/hudi/util/AvroConvertor.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS91dGlsL0F2cm9Db252ZXJ0b3IuamF2YQ==) | `0.00% <0.00%> (-82.36%)` | `0.00% <0.00%> (-6.00%)` | |
   | [...c/main/java/org/apache/hudi/util/StreamerUtil.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS91dGlsL1N0cmVhbWVyVXRpbC5qYXZh) | `54.23% <0.00%> (-3.39%)` | `16.00% <0.00%> (-2.00%)` | |
   | [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `71.08% <0.00%> (-0.35%)` | `55.00% <0.00%> (-1.00%)` | |
   | [.../hudi/table/format/mor/MergeOnReadInputFormat.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS90YWJsZS9mb3JtYXQvbW9yL01lcmdlT25SZWFkSW5wdXRGb3JtYXQuamF2YQ==) | `75.00% <0.00%> (ø)` | `18.00% <0.00%> (ø%)` | |
   


-- 
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.

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

[GitHub] [hudi] codecov-commenter commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-825580328


   # [Codecov](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2868](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3928b17) into [master](https://codecov.io/gh/apache/hudi/commit/4a3431866d995fd877a067967457591a0bc8847d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a34318) will **increase** coverage by `16.67%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hudi/pull/2868/graphs/tree.svg?width=650&height=150&src=pr&token=VTTXabwbs2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #2868       +/-   ##
   =============================================
   + Coverage     53.01%   69.68%   +16.67%     
   + Complexity     3744      373     -3371     
   =============================================
     Files           488       54      -434     
     Lines         23435     1996    -21439     
     Branches       2500      236     -2264     
   =============================================
   - Hits          12423     1391    -11032     
   + Misses         9913      473     -9440     
   + Partials       1099      132      -967     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | hudicli | `?` | `?` | |
   | hudiclient | `?` | `?` | |
   | hudicommon | `?` | `?` | |
   | hudiflink | `?` | `?` | |
   | hudihadoopmr | `?` | `?` | |
   | hudisparkdatasource | `?` | `?` | |
   | hudisync | `?` | `?` | |
   | huditimelineservice | `?` | `?` | |
   | hudiutilities | `69.68% <ø> (-0.06%)` | `373.00 <ø> (-1.00)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `71.08% <0.00%> (-0.35%)` | `55.00% <0.00%> (-1.00%)` | |
   | [...apache/hudi/common/util/collection/ArrayUtils.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvY29sbGVjdGlvbi9BcnJheVV0aWxzLmphdmE=) | | | |
   | [...udi/common/util/queue/BoundedInMemoryExecutor.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvcXVldWUvQm91bmRlZEluTWVtb3J5RXhlY3V0b3IuamF2YQ==) | | | |
   | [...n/java/org/apache/hudi/hadoop/InputSplitUtils.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1oYWRvb3AtbXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGFkb29wL0lucHV0U3BsaXRVdGlscy5qYXZh) | | | |
   | [...g/apache/hudi/sink/partitioner/BucketAssigner.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS9zaW5rL3BhcnRpdGlvbmVyL0J1Y2tldEFzc2lnbmVyLmphdmE=) | | | |
   | [...apache/hudi/timeline/service/handlers/Handler.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvaGFuZGxlcnMvSGFuZGxlci5qYXZh) | | | |
   | [...org/apache/hudi/common/config/TypedProperties.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2NvbmZpZy9UeXBlZFByb3BlcnRpZXMuamF2YQ==) | | | |
   | [...rg/apache/hudi/common/model/HoodieFileGroupId.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL21vZGVsL0hvb2RpZUZpbGVHcm91cElkLmphdmE=) | | | |
   | [...g/apache/hudi/common/function/FunctionWrapper.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2Z1bmN0aW9uL0Z1bmN0aW9uV3JhcHBlci5qYXZh) | | | |
   | [...pache/hudi/common/config/HoodieMetadataConfig.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL2NvbmZpZy9Ib29kaWVNZXRhZGF0YUNvbmZpZy5qYXZh) | | | |
   | ... and [417 more](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828323317


   I have two questions:
   
   1) The lowest Flink version we supported is 1.12.x?
   2) Can we provide an e2e demo and documentation to show the usage of the flink streamer via jar-mode, just like delta-streamer, it should be out of the box;
   
   I tried it, but missed the dependencies of the Kafka connector. Can we make the new flink streamer peer to the delta streamer?


-- 
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.

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

[GitHub] [hudi] MyLanPangzi commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
MyLanPangzi commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-833493033


   > > see https://issues.apache.org/jira/browse/HUDI-1872
   > 
   > @MyLanPangzi Are you interested in this ticket? To continue refactoring.
   
   yes, I have already made part of it in my local repo.


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-829065787


   > > > > > Fine, let's keep them for a time.
   > > > > 
   > > > > 
   > > > > I do not object to deleting the legacy code. I just suggest we'd better add the Kafka connector into the bundle. And let it be a completed function. Then, we can delete the legacy code and use the new Flink streamer.
   > > > 
   > > > 
   > > > I agree the Flink streamer should bundle with a kafka jar, but the hudi-flink-bundle should not include kafka, because many people include the bundle jar in the pom directly, we should keep the jar as small as possible to avoid unnecessary jar conflicts.
   > > > Maybe we should move the streamer out of the hudi-flink module into the hudi-utilities module.
   > > 
   > > 
   > > +1, it's what I mean.
   > 
   > That's cool, we can do the move in another PR and this PR can merge first i think.
   
   OK, please file a ticket to track this work.


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-834023836


   > > > see https://issues.apache.org/jira/browse/HUDI-1872
   > > 
   > > 
   > > @MyLanPangzi Are you interested in this ticket? To continue refactoring.
   > 
   > yes, I have already made part of it in my local repo.
   
   sg. 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.

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

[GitHub] [hudi] danny0405 commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828941274


   Fine, let's keep them for a time.


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-833459923


   > see https://issues.apache.org/jira/browse/HUDI-1872
   
   @MyLanPangzi Are you interested in this ticket? To continue refactoring.


-- 
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.

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

[GitHub] [hudi] danny0405 commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-829083949


   see https://issues.apache.org/jira/browse/HUDI-1872


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828408491


   > > I have two questions:
   > > 
   > > 1. The lowest Flink version we supported is 1.12.x?
   > > 2. Can we provide an e2e demo and documentation to show the usage of the flink streamer via jar-mode, just like delta-streamer, it should be out of the box;
   > > 
   > > I tried it, but missed the dependencies of the Kafka connector. Can we make the new flink streamer peer to the delta streamer?
   > 
   > Yes, people would only use flink 1.12.x code, the code to remove is not because of flink version, it's because the logic is totally redundant. Remove to avoid cofusion, because i found some people use the legacy code with poor performance.
   
   Although I know that many users are currently testing based on 1.12, the threshold we set for many users of older versions is very high. Pray that they are willing to upgrade the Flink version in order to use hudi. In fact, I personally think that the biggest improvement of the new implementation lies in the bucket assigner. As for other points, we could have found a solution (although it does not seem very elegant). Well, I don't have to worry about the Flink version anymore, and I don't have time to pay attention to the old implementation.
   
   > I still think we should not include a kafka connector into the delta streamer, on one complains the missing of it, based on the users i see.
   
   I still insist that we need to include kafka-related dependencies. If you look back at the HoodieFlinkStreamerV2 class. What is it in essence? It is just a program written using Flink DataStream API, which is specific (Kafka -> Hudi), not plug-in-oriented or abstract-oriented. For a specific Flink program, we should provide users with an Uber(fat) Jar. Instead of letting users pay attention to details and pay additional costs. Otherwise, why don't we make the source universal?
   


-- 
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.

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

[GitHub] [hudi] yanghua commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828962709


   > Fine, let's keep them for a time.
   
   I do not object to deleting the legacy code. I just suggest we'd better add the Kafka connector into the bundle. And let it be a completed function. Then, we can delete the legacy code and use the new Flink streamer.


-- 
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.

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

[GitHub] [hudi] danny0405 edited a comment on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 edited a comment on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828336262


   > I have two questions:
   > 
   > 1. The lowest Flink version we supported is 1.12.x?
   > 2. Can we provide an e2e demo and documentation to show the usage of the flink streamer via jar-mode, just like delta-streamer, it should be out of the box;
   > 
   > I tried it, but missed the dependencies of the Kafka connector. Can we make the new flink streamer peer to the delta streamer?
   
   Yes, people would only use flink 1.12.x code, the code to remove is not because of flink version, it's because the logic is totally redundant. Remove to avoid cofusion, because i found some people use the legacy code with poor performace.
   
   I still think we should not include a kafka connector into the delta streamer, on one complains the missing of it, based on the users i see.


-- 
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.

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

[GitHub] [hudi] danny0405 commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828336262


   > I have two questions:
   > 
   > 1. The lowest Flink version we supported is 1.12.x?
   > 2. Can we provide an e2e demo and documentation to show the usage of the flink streamer via jar-mode, just like delta-streamer, it should be out of the box;
   > 
   > I tried it, but missed the dependencies of the Kafka connector. Can we make the new flink streamer peer to the delta streamer?
   
   Yes, people would only use flink 1.12.x code, the code to remove is not because of flink version, it's because the logic is totally redundant.
   
   I still think we should not include a kafka connector into the delta streamer, on one complains the missing of it, based on the users i see.


-- 
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.

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

[GitHub] [hudi] codecov-commenter edited a comment on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-825580328


   # [Codecov](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#2868](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3928b17) into [master](https://codecov.io/gh/apache/hudi/commit/4a3431866d995fd877a067967457591a0bc8847d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4a34318) will **increase** coverage by `0.52%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hudi/pull/2868/graphs/tree.svg?width=650&height=150&src=pr&token=VTTXabwbs2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #2868      +/-   ##
   ============================================
   + Coverage     53.01%   53.53%   +0.52%     
   + Complexity     3744     3732      -12     
   ============================================
     Files           488      481       -7     
     Lines         23435    23140     -295     
     Branches       2500     2468      -32     
   ============================================
   - Hits          12423    12388      -35     
   + Misses         9913     9653     -260     
     Partials       1099     1099              
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | hudicli | `39.53% <ø> (ø)` | `220.00 <ø> (ø)` | |
   | hudiclient | `∅ <ø> (∅)` | `0.00 <ø> (ø)` | |
   | hudicommon | `50.65% <ø> (ø)` | `1976.00 <ø> (ø)` | |
   | hudiflink | `63.24% <0.00%> (+4.12%)` | `524.00 <0.00> (-11.00)` | :arrow_up: |
   | hudihadoopmr | `33.33% <ø> (ø)` | `198.00 <ø> (ø)` | |
   | hudisparkdatasource | `73.33% <ø> (ø)` | `237.00 <ø> (ø)` | |
   | hudisync | `46.10% <ø> (ø)` | `142.00 <ø> (ø)` | |
   | huditimelineservice | `64.36% <ø> (ø)` | `62.00 <ø> (ø)` | |
   | hudiutilities | `69.68% <ø> (-0.06%)` | `373.00 <ø> (-1.00)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2868?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../org/apache/hudi/streamer/HoodieFlinkStreamer.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS9zdHJlYW1lci9Ib29kaWVGbGlua1N0cmVhbWVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [.../main/java/org/apache/hudi/util/AvroConvertor.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS91dGlsL0F2cm9Db252ZXJ0b3IuamF2YQ==) | `0.00% <0.00%> (-82.36%)` | `0.00% <0.00%> (-6.00%)` | |
   | [...c/main/java/org/apache/hudi/util/StreamerUtil.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS91dGlsL1N0cmVhbWVyVXRpbC5qYXZh) | `54.23% <0.00%> (-3.39%)` | `16.00% <0.00%> (-2.00%)` | |
   | [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `71.08% <0.00%> (-0.35%)` | `55.00% <0.00%> (-1.00%)` | |
   | [.../hudi/table/format/mor/MergeOnReadInputFormat.java](https://codecov.io/gh/apache/hudi/pull/2868/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS90YWJsZS9mb3JtYXQvbW9yL01lcmdlT25SZWFkSW5wdXRGb3JtYXQuamF2YQ==) | `75.00% <0.00%> (ø)` | `18.00% <0.00%> (ø%)` | |
   


-- 
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.

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

[GitHub] [hudi] MyLanPangzi removed a comment on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
MyLanPangzi removed a comment on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828259208


   @yanghua Hi，i triggered the ci and it's passed.


-- 
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.

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

[GitHub] [hudi] danny0405 commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-828972395


   > > Fine, let's keep them for a time.
   > 
   > I do not object to deleting the legacy code. I just suggest we'd better add the Kafka connector into the bundle. And let it be a completed function. Then, we can delete the legacy code and use the new Flink streamer.
   
   I agree the Flink streamer should bundle with a kafka jar, but the hudi-flink-bundle should not include kafka, because many people include the bundle jar in the pom directly, we should keep the jar as small as possible to avoid unnecessary jar conflicts.
   
   Maybe we should move the streamer out of the hudi-flink module into the hudi-utilities module.


-- 
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.

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

[GitHub] [hudi] danny0405 commented on pull request #2868: [HUDI-1821] Remove legacy code for Flink writer

Posted by GitBox <gi...@apache.org>.
danny0405 commented on pull request #2868:
URL: https://github.com/apache/hudi/pull/2868#issuecomment-829033000


   > > > > Fine, let's keep them for a time.
   > > > 
   > > > 
   > > > I do not object to deleting the legacy code. I just suggest we'd better add the Kafka connector into the bundle. And let it be a completed function. Then, we can delete the legacy code and use the new Flink streamer.
   > > 
   > > 
   > > I agree the Flink streamer should bundle with a kafka jar, but the hudi-flink-bundle should not include kafka, because many people include the bundle jar in the pom directly, we should keep the jar as small as possible to avoid unnecessary jar conflicts.
   > > Maybe we should move the streamer out of the hudi-flink module into the hudi-utilities module.
   > 
   > +1, it's what I mean.
   
   That's cool, we can do the move in another PR and this PR can merge first i think.


-- 
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.

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