apache / pinot

Apache Pinot - A realtime distributed OLAP datastore
https://pinot.apache.org/
Apache License 2.0
5.17k stars 1.21k forks source link

#13049 Refactored RecordTransformer & merged RecordEnricher #13086

Open deepthi912 opened 1 week ago

deepthi912 commented 1 week ago

1) Removed RecordEnricher 2) Changed the package of pipeline, factory and Config of RecordEnricher to "org.apache.pinot.plugin.record.enricher" 3) Used transform in RecordTransformer to serve the purpose of enrich in RecordEnricher.

codecov-commenter commented 1 week ago

Codecov Report

Attention: Patch coverage is 50.81967% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 62.18%. Comparing base (59551e4) to head (0e02293). Report is 433 commits behind head on master.

Files Patch % Lines
...ache/pinot/segment/local/utils/IngestionUtils.java 29.41% 11 Missing and 1 partial :warning:
...he/pinot/common/utils/config/TableConfigUtils.java 45.45% 4 Missing and 2 partials :warning:
...he/pinot/segment/local/utils/TableConfigUtils.java 63.63% 1 Missing and 3 partials :warning:
...plugin/record/enricher/RecordEnricherPipeline.java 25.00% 2 Missing and 1 partial :warning:
...l/realtime/converter/RealtimeSegmentConverter.java 75.00% 2 Missing :warning:
...lugin/record/enricher/clp/CLPEncodingEnricher.java 0.00% 1 Missing :warning:
...cord/enricher/function/CustomFunctionEnricher.java 0.00% 1 Missing :warning:
...ent/local/recordtransformer/RecordTransformer.java 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #13086 +/- ## ============================================ + Coverage 61.75% 62.18% +0.42% + Complexity 207 198 -9 ============================================ Files 2436 2515 +79 Lines 133233 137858 +4625 Branches 20636 21324 +688 ============================================ + Hits 82274 85722 +3448 - Misses 44911 45745 +834 - Partials 6048 6391 +343 ``` | [Flag](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: | | [integration](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: | | [integration1](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <0.00%> (-0.01%)` | :arrow_down: | | [integration2](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <0.00%> (ø)` | | | [java-11](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.14% <50.81%> (+0.43%)` | :arrow_up: | | [java-21](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.05% <50.81%> (+0.43%)` | :arrow_up: | | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.16% <50.81%> (+0.41%)` | :arrow_up: | | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.04% <50.81%> (+34.31%)` | :arrow_up: | | [temurin](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.18% <50.81%> (+0.42%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.17% <50.81%> (+0.42%)` | :arrow_up: | | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.81% <8.19%> (-0.08%)` | :arrow_down: | | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/13086/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.78% <42.62%> (+0.04%)` | :arrow_up: | 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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

deepthi912 commented 1 week ago

Sure, I wanted to do that too. Will look and see where I can improve further

deepthi912 commented 1 week ago

@Jackie-Jiang I noticed that registery, pipeline have static methods which are used by some of the classes. These static methods are calling RecordEnricherFactory which is again implemented by function and clp Enricher classes. I tried to check if I could squeeze in anywhere but no right spot. Not sure if by just removing RecordEnricher, we can make any big difference in refactoring though.

Jackie-Jiang commented 5 days ago

Basically the idea is to re-do #12243 and integrate CLP transform as a RecordTransformer