You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "sajjad-moradi (via GitHub)" <gi...@apache.org> on 2023/06/07 19:14:58 UTC

[GitHub] [pinot] sajjad-moradi opened a new pull request, #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

sajjad-moradi opened a new pull request, #10869:
URL: https://github.com/apache/pinot/pull/10869

   Solves the issue https://github.com/apache/pinot/issues/10868
   
   I updated the minion purge integration tests to add a scenario in which the segments are built with old schema. Without the fix that unit test fails.
   
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #10869:
URL: https://github.com/apache/pinot/pull/10869#issuecomment-1581527890

   This doesn't seem to be the correct fix.
   
   The real problem is that we are using the table config from the ZK and schema from the segment file (we picked this way before because schema might not be stored in ZK when this task was implemented). Instead, we should also read the schema from the ZK so that schema is in-sync with the table config. You may read the schema in `PurgeTaskExecutor` and pass it into the `SegmentPurger`, then use it to create the `SegmentGeneratorConfig`.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] codecov-commenter commented on pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10869:
URL: https://github.com/apache/pinot/pull/10869#issuecomment-1581425361

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/10869?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   > Merging [#10869](https://app.codecov.io/gh/apache/pinot/pull/10869?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (4bf056c) into [master](https://app.codecov.io/gh/apache/pinot/commit/3b2e31cfad6d327ca41021e091812f0f7e17e453?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (3b2e31c) will **decrease** coverage by `20.84%`.
   > The diff coverage is `0.00%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #10869       +/-   ##
   =============================================
   - Coverage     34.52%   13.68%   -20.84%     
   + Complexity      462      439       -23     
   =============================================
     Files          2174     2120       -54     
     Lines        116848   114354     -2494     
     Branches      17692    17391      -301     
   =============================================
   - Hits          40339    15649    -24690     
   - Misses        72991    97421    +24430     
   + Partials       3518     1284     -2234     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests2 | `13.68% <0.00%> (-0.02%)` | :arrow_down: |
   
   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.
   
   | [Impacted Files](https://app.codecov.io/gh/apache/pinot/pull/10869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [...inot/controller/helix/ControllerRequestClient.java](https://app.codecov.io/gh/apache/pinot/pull/10869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9Db250cm9sbGVyUmVxdWVzdENsaWVudC5qYXZh) | `36.18% <0.00%> (-19.99%)` | :arrow_down: |
   | [...ment/creator/impl/SegmentColumnarIndexCreator.java](https://app.codecov.io/gh/apache/pinot/pull/10869?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2NyZWF0b3IvaW1wbC9TZWdtZW50Q29sdW1uYXJJbmRleENyZWF0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   
   ... and [774 files with indirect coverage changes](https://app.codecov.io/gh/apache/pinot/pull/10869/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] PrachiKhobragade commented on a diff in pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "PrachiKhobragade (via GitHub)" <gi...@apache.org>.
PrachiKhobragade commented on code in PR #10869:
URL: https://github.com/apache/pinot/pull/10869#discussion_r1222170641


##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PurgeMinionClusterIntegrationTest.java:
##########
@@ -96,23 +98,27 @@ public void setUp()
     TableConfig purgeDeltaPassedTableConfig = createOfflineTableConfig();
     purgeDeltaPassedTableConfig.setTaskConfig(getPurgeTaskConfig());
 
+    setTableName(PURGE_OLD_SEGMENTS_WITH_NEW_INDICES_TABLE);
+    TableConfig purgeOldSegmentsWithNewIndicesTableConfig = createOfflineTableConfig();
+    purgeOldSegmentsWithNewIndicesTableConfig.setTaskConfig(getPurgeTaskConfig());
+
     addTableConfig(purgeTableConfig);
     addTableConfig(purgeDeltaPassedTableConfig);
     addTableConfig(purgeDeltaNotPassedTableConfig);
+    addTableConfig(purgeOldSegmentsWithNewIndicesTableConfig);
 
     // Unpack the Avro files
     List<File> avroFiles = unpackAvroData(_tempDir);
 
-    // Create and upload segments
-    ClusterIntegrationTestUtils.buildSegmentsFromAvro(avroFiles, purgeTableConfig, schema, 0, _segmentDir1, _tarDir1);

Review Comment:
   Why did we consolidate everything to same directory?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10869:
URL: https://github.com/apache/pinot/pull/10869#discussion_r1222276612


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/purge/PurgeTaskExecutor.java:
##########
@@ -44,16 +45,18 @@ protected SegmentConversionResult convert(PinotTaskConfig pinotTaskConfig, File
     String tableNameWithType = configs.get(MinionConstants.TABLE_NAME_KEY);
     String rawTableName = TableNameBuilder.extractRawTableName(tableNameWithType);
 
-    TableConfig tableConfig = getTableConfig(tableNameWithType);
     SegmentPurger.RecordPurgerFactory recordPurgerFactory = MINION_CONTEXT.getRecordPurgerFactory();
     SegmentPurger.RecordPurger recordPurger =
         recordPurgerFactory != null ? recordPurgerFactory.getRecordPurger(rawTableName) : null;
     SegmentPurger.RecordModifierFactory recordModifierFactory = MINION_CONTEXT.getRecordModifierFactory();
     SegmentPurger.RecordModifier recordModifier =
         recordModifierFactory != null ? recordModifierFactory.getRecordModifier(rawTableName) : null;
 
+    TableConfig tableConfig = getTableConfig(tableNameWithType);
+    Schema schema = getSchema(rawTableName);

Review Comment:
   ```suggestion
       Schema schema = getSchema(tableNameWithType);
   ```



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #10869:
URL: https://github.com/apache/pinot/pull/10869#discussion_r1222241882


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/purge/PurgeTaskExecutor.java:
##########
@@ -44,16 +45,19 @@ protected SegmentConversionResult convert(PinotTaskConfig pinotTaskConfig, File
     String tableNameWithType = configs.get(MinionConstants.TABLE_NAME_KEY);
     String rawTableName = TableNameBuilder.extractRawTableName(tableNameWithType);
 
-    TableConfig tableConfig = getTableConfig(tableNameWithType);
     SegmentPurger.RecordPurgerFactory recordPurgerFactory = MINION_CONTEXT.getRecordPurgerFactory();
     SegmentPurger.RecordPurger recordPurger =
         recordPurgerFactory != null ? recordPurgerFactory.getRecordPurger(rawTableName) : null;
     SegmentPurger.RecordModifierFactory recordModifierFactory = MINION_CONTEXT.getRecordModifierFactory();
     SegmentPurger.RecordModifier recordModifier =
         recordModifierFactory != null ? recordModifierFactory.getRecordModifier(rawTableName) : null;
 
+    TableConfig tableConfig = getTableConfig(tableNameWithType);
+    String schemaName = tableConfig.getValidationConfig().getSchemaName();
+    Schema schema = getSchema(schemaName);

Review Comment:
   `getSchema()` takes `tableName` instead of `schemaName`. `schemaName` might not be configured in the table config.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] mcvsubbu commented on pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "mcvsubbu (via GitHub)" <gi...@apache.org>.
mcvsubbu commented on PR #10869:
URL: https://github.com/apache/pinot/pull/10869#issuecomment-1581532821

   > This doesn't seem to be the correct fix.
   > 
   > The real problem is that we are using the table config from the ZK and schema from the segment file (we picked this way before because schema might not be stored in ZK when this task was implemented). Instead, we should also read the schema from the ZK so that schema is in-sync with the table config. You may read the schema in `PurgeTaskExecutor` and pass it into the `SegmentPurger`, then use it to create the `SegmentGeneratorConfig`.
   
   Minion purge task can purge data from old segments, which may have been built with an older schema (minus some columns). I thought the idea was to keep the old segments without the new column (and let the server auto-load default value into new column instead of baking it into the segment).


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #10869:
URL: https://github.com/apache/pinot/pull/10869#issuecomment-1581585748

   > There is also a case of older tables (that exist even now) when schema was optional for offline tables
   
   Good point. In that case, we can use the schema from the segment


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] mcvsubbu commented on pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "mcvsubbu (via GitHub)" <gi...@apache.org>.
mcvsubbu commented on PR #10869:
URL: https://github.com/apache/pinot/pull/10869#issuecomment-1581580573

   There is also a case of older tables (that exist even now) when schema was optional for offline tables


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] sajjad-moradi commented on pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "sajjad-moradi (via GitHub)" <gi...@apache.org>.
sajjad-moradi commented on PR #10869:
URL: https://github.com/apache/pinot/pull/10869#issuecomment-1581551557

   > > This doesn't seem to be the correct fix.
   > > The real problem is that we are using the table config from the ZK and schema from the segment file (we picked this way before because schema might not be stored in ZK when this task was implemented). Instead, we should also read the schema from the ZK so that schema is in-sync with the table config. You may read the schema in `PurgeTaskExecutor` and pass it into the `SegmentPurger`, then use it to create the `SegmentGeneratorConfig`.
   > 
   > Minion purge task can purge data from old segments, which may have been built with an older schema (minus some columns). I thought the idea was to keep the old segments without the new column (and let the server auto-load default value into new column instead of baking it into the segment).
   
   Subbu, with fetching schema from ZK, once the segments are built by Minion and pushed to deep store, the Servers don't need to do extra work for loading default values.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] snleee commented on a diff in pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "snleee (via GitHub)" <gi...@apache.org>.
snleee commented on code in PR #10869:
URL: https://github.com/apache/pinot/pull/10869#discussion_r1222291763


##########
pinot-core/src/main/java/org/apache/pinot/core/minion/SegmentPurger.java:
##########
@@ -79,7 +82,7 @@ public File purgeSegment()
         return null;
       }
 
-      SegmentGeneratorConfig config = new SegmentGeneratorConfig(_tableConfig, segmentMetadata.getSchema());
+      SegmentGeneratorConfig config = new SegmentGeneratorConfig(_tableConfig, _schema);

Review Comment:
   If `_schema = null`, probably it's safer to fall back to `segmentMetadata.getSchema()`?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] sajjad-moradi commented on a diff in pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "sajjad-moradi (via GitHub)" <gi...@apache.org>.
sajjad-moradi commented on code in PR #10869:
URL: https://github.com/apache/pinot/pull/10869#discussion_r1222252119


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/purge/PurgeTaskExecutor.java:
##########
@@ -44,16 +45,19 @@ protected SegmentConversionResult convert(PinotTaskConfig pinotTaskConfig, File
     String tableNameWithType = configs.get(MinionConstants.TABLE_NAME_KEY);
     String rawTableName = TableNameBuilder.extractRawTableName(tableNameWithType);
 
-    TableConfig tableConfig = getTableConfig(tableNameWithType);
     SegmentPurger.RecordPurgerFactory recordPurgerFactory = MINION_CONTEXT.getRecordPurgerFactory();
     SegmentPurger.RecordPurger recordPurger =
         recordPurgerFactory != null ? recordPurgerFactory.getRecordPurger(rawTableName) : null;
     SegmentPurger.RecordModifierFactory recordModifierFactory = MINION_CONTEXT.getRecordModifierFactory();
     SegmentPurger.RecordModifier recordModifier =
         recordModifierFactory != null ? recordModifierFactory.getRecordModifier(rawTableName) : null;
 
+    TableConfig tableConfig = getTableConfig(tableNameWithType);
+    String schemaName = tableConfig.getValidationConfig().getSchemaName();
+    Schema schema = getSchema(schemaName);

Review Comment:
   I'm not sure about all other places in code, but at least in this integration test, one schema is used for all three tables.
   I will refactor the integration test.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] sajjad-moradi commented on a diff in pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "sajjad-moradi (via GitHub)" <gi...@apache.org>.
sajjad-moradi commented on code in PR #10869:
URL: https://github.com/apache/pinot/pull/10869#discussion_r1222188549


##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PurgeMinionClusterIntegrationTest.java:
##########
@@ -96,23 +98,27 @@ public void setUp()
     TableConfig purgeDeltaPassedTableConfig = createOfflineTableConfig();
     purgeDeltaPassedTableConfig.setTaskConfig(getPurgeTaskConfig());
 
+    setTableName(PURGE_OLD_SEGMENTS_WITH_NEW_INDICES_TABLE);
+    TableConfig purgeOldSegmentsWithNewIndicesTableConfig = createOfflineTableConfig();
+    purgeOldSegmentsWithNewIndicesTableConfig.setTaskConfig(getPurgeTaskConfig());
+
     addTableConfig(purgeTableConfig);
     addTableConfig(purgeDeltaPassedTableConfig);
     addTableConfig(purgeDeltaNotPassedTableConfig);
+    addTableConfig(purgeOldSegmentsWithNewIndicesTableConfig);
 
     // Unpack the Avro files
     List<File> avroFiles = unpackAvroData(_tempDir);
 
-    // Create and upload segments
-    ClusterIntegrationTestUtils.buildSegmentsFromAvro(avroFiles, purgeTableConfig, schema, 0, _segmentDir1, _tarDir1);

Review Comment:
   That wasn't needed. The schema and table configs for all those three tables were the same, so were the segments that were built. So there's no need to build the segments three times.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] sajjad-moradi commented on a diff in pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "sajjad-moradi (via GitHub)" <gi...@apache.org>.
sajjad-moradi commented on code in PR #10869:
URL: https://github.com/apache/pinot/pull/10869#discussion_r1222252119


##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/purge/PurgeTaskExecutor.java:
##########
@@ -44,16 +45,19 @@ protected SegmentConversionResult convert(PinotTaskConfig pinotTaskConfig, File
     String tableNameWithType = configs.get(MinionConstants.TABLE_NAME_KEY);
     String rawTableName = TableNameBuilder.extractRawTableName(tableNameWithType);
 
-    TableConfig tableConfig = getTableConfig(tableNameWithType);
     SegmentPurger.RecordPurgerFactory recordPurgerFactory = MINION_CONTEXT.getRecordPurgerFactory();
     SegmentPurger.RecordPurger recordPurger =
         recordPurgerFactory != null ? recordPurgerFactory.getRecordPurger(rawTableName) : null;
     SegmentPurger.RecordModifierFactory recordModifierFactory = MINION_CONTEXT.getRecordModifierFactory();
     SegmentPurger.RecordModifier recordModifier =
         recordModifierFactory != null ? recordModifierFactory.getRecordModifier(rawTableName) : null;
 
+    TableConfig tableConfig = getTableConfig(tableNameWithType);
+    String schemaName = tableConfig.getValidationConfig().getSchemaName();
+    Schema schema = getSchema(schemaName);

Review Comment:
   I'm not sure about all other places in code, but at least in this integration test, one schema is used for all three tables.
   I will refactor this.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] sajjad-moradi merged pull request #10869: Bug Fix: Segment Purger cannot purge old segments after schema evolution

Posted by "sajjad-moradi (via GitHub)" <gi...@apache.org>.
sajjad-moradi merged PR #10869:
URL: https://github.com/apache/pinot/pull/10869


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org