You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "yashmayya (via GitHub)" <gi...@apache.org> on 2024/08/21 13:57:26 UTC
[PR] Log table config and schema updates [pinot]
yashmayya opened a new pull request, #13867:
URL: https://github.com/apache/pinot/pull/13867
- Currently, changes to table configs and schemas aren't logged. This makes debugging the history of a table difficult.
- This minor patch adds info logs to the `addSchema`, `updateSchema`, `addTable`, `updateTableConfig` APIs.
- Sample logs:
```
Successfully created new schema: {
"schemaName" : "testSchema",
"enableColumnBasedNullHandling" : false,
"dimensionFieldSpecs" : [ {
"name" : "OldColumn",
"dataType" : "INT",
"notNull" : false
} ]
}
```
```
Successfully updated schema: testSchema with new value: {
"schemaName" : "testSchema",
"enableColumnBasedNullHandling" : false,
"dimensionFieldSpecs" : [ {
"name" : "NewColumn",
"dataType" : "STRING",
"notNull" : false
}, {
"name" : "OldColumn",
"dataType" : "INT",
"notNull" : false
} ]
}
```
```
Successfully added table with name: airlineStats_OFFLINE and config: {"tableName":"airlineStats_OFFLINE","tableType":"OFFLINE","segmentsConfig":{"timeColumnName":"DaysSinceEpoch","timeType":"DAYS","replication":"1","segmentPushType":"APPEND","minimizeDataMovement":false,"segmentAssignmentStrategy":"BalanceNumSegmentAssignmentStrategy"},"tenants":{"broker":"DefaultTenant","server":"DefaultTenant"},"tableIndexConfig":{"aggregateMetrics":false,"starTreeIndexConfigs":[{"dimensionsSplitOrder":["AirlineID","Origin","Dest"],"functionColumnPairs":["COUNT__*","MAX__ArrDelay"],"maxLeafRecords":10}],"enableDefaultStarTree":false,"rangeIndexVersion":2,"loadMode":"MMAP","tierOverwrites":{"hotTier":{"starTreeIndexConfigs":[{"dimensionsSplitOrder":["Carrier","CancellationCode","Origin","Dest"],"skipStarNodeCreationForDimensions":[],"functionColumnPairs":["MAX__CarrierDelay","AVG__CarrierDelay"],"maxLeafRecords":10}]},"coldTier":{"starTreeIndexConfigs":[]}},"nullHandlingEnabled":false,"optimizeDi
ctionary":false,"autoGeneratedInvertedIndex":false,"enableDynamicStarTreeCreation":true,"columnMajorSegmentBuilderEnabled":true,"optimizeDictionaryForMetrics":false,"noDictionarySizeRatioThreshold":0.85,"createInvertedIndexDuringSegmentGeneration":false},"metadata":{"customConfigs":{}},"fieldConfigList":[{"name":"ts","encodingType":"DICTIONARY","indexType":"TIMESTAMP","indexTypes":["TIMESTAMP"],"timestampConfig":{"granularities":["DAY","WEEK","MONTH"]},"indexes":null,"tierOverwrites":null},{"name":"ArrTimeBlk","encodingType":"DICTIONARY","indexTypes":[],"indexes":{"inverted":{"enabled":"true"}},"tierOverwrites":{"hotTier":{"encodingType":"DICTIONARY","indexes":{"bloom":{"enabled":"true"}}},"coldTier":{"encodingType":"RAW","indexes":{"text":{"enabled":"true"}}}}}],"ingestionConfig":{"transformConfigs":[{"columnName":"ts","transformFunction":"fromEpochDays(DaysSinceEpoch)"},{"columnName":"tsRaw","transformFunction":"fromEpochDays(DaysSinceEpoch)"}],"segmentTimeValueCheck":true,"rowTim
eValueCheck":false,"continueOnError":false},"tierConfigs":[{"name":"hotTier","segmentSelectorType":"time","segmentAge":"3130d","storageType":"pinot_server","serverTag":"DefaultTenant_OFFLINE"},{"name":"coldTier","segmentSelectorType":"time","segmentAge":"3140d","storageType":"pinot_server","serverTag":"DefaultTenant_OFFLINE"}],"isDimTable":false}
```
```
Successfully updated table config for table: airlineStats_OFFLINE with new config: {"tableName":"airlineStats_OFFLINE","tableType":"OFFLINE","segmentsConfig":{"timeColumnName":"DaysSinceEpoch","timeType":"DAYS","replication":"1","segmentPushType":"APPEND","minimizeDataMovement":false,"segmentAssignmentStrategy":"BalanceNumSegmentAssignmentStrategy"},"tenants":{"broker":"DefaultTenant","server":"DefaultTenant"},"tableIndexConfig":{"aggregateMetrics":false,"starTreeIndexConfigs":[{"dimensionsSplitOrder":["AirlineID","Origin","Dest"],"functionColumnPairs":["COUNT__*","MAX__ArrDelay"],"maxLeafRecords":10}],"enableDefaultStarTree":false,"rangeIndexVersion":2,"loadMode":"MMAP","tierOverwrites":{"hotTier":{"starTreeIndexConfigs":[{"dimensionsSplitOrder":["Carrier","CancellationCode","Origin","Dest"],"skipStarNodeCreationForDimensions":[],"functionColumnPairs":["MAX__CarrierDelay","AVG__CarrierDelay"],"maxLeafRecords":10}]},"coldTier":{"starTreeIndexConfigs":[]}},"nullHandlingEnabled":fal
se,"optimizeDictionary":false,"autoGeneratedInvertedIndex":false,"enableDynamicStarTreeCreation":false,"columnMajorSegmentBuilderEnabled":true,"optimizeDictionaryForMetrics":false,"noDictionarySizeRatioThreshold":0.85,"createInvertedIndexDuringSegmentGeneration":false},"metadata":{"customConfigs":{}},"fieldConfigList":[{"name":"ts","encodingType":"DICTIONARY","indexType":"TIMESTAMP","indexTypes":["TIMESTAMP"],"timestampConfig":{"granularities":["DAY","WEEK","MONTH"]},"indexes":null,"tierOverwrites":null},{"name":"ArrTimeBlk","encodingType":"DICTIONARY","indexTypes":[],"indexes":{"inverted":{"enabled":"true"}},"tierOverwrites":{"hotTier":{"encodingType":"DICTIONARY","indexes":{"bloom":{"enabled":"true"}}},"coldTier":{"encodingType":"RAW","indexes":{"text":{"enabled":"true"}}}}}],"ingestionConfig":{"transformConfigs":[{"columnName":"ts","transformFunction":"fromEpochDays(DaysSinceEpoch)"},{"columnName":"tsRaw","transformFunction":"fromEpochDays(DaysSinceEpoch)"}],"segmentTimeValueChec
k":true,"rowTimeValueCheck":false,"continueOnError":false},"tierConfigs":[{"name":"hotTier","segmentSelectorType":"time","segmentAge":"3130d","storageType":"pinot_server","serverTag":"DefaultTenant_OFFLINE"},{"name":"coldTier","segmentSelectorType":"time","segmentAge":"3140d","storageType":"pinot_server","serverTag":"DefaultTenant_OFFLINE"}],"isDimTable":false}
```
--
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
Re: [PR] Log table config and schema updates [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on PR #13867:
URL: https://github.com/apache/pinot/pull/13867#issuecomment-2302266977
I think let's keep it to what it is currently since the logs are emitted after the Helix / ZK state is updated and that is user visible. Also, I guess users should already be familiar with the async nature from the current success API responses.
--
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
Re: [PR] Log table config and schema updates [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on PR #13867:
URL: https://github.com/apache/pinot/pull/13867#issuecomment-2302141342
Two suggestions:
1. Can we unify logs (apply the same change I suggested for schema in table config)?
2. Can we create a similar log when tables are deleted. If possible, using the same log pattern so the same regex can be used.
--
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
Re: [PR] Log table config and schema updates [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #13867:
URL: https://github.com/apache/pinot/pull/13867#issuecomment-2302298095
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/13867?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `0%` with `6 lines` in your changes missing coverage. Please review.
> Project coverage is 0.00%. Comparing base [(`59551e4`)](https://app.codecov.io/gh/apache/pinot/commit/59551e45224f1535c4863fd577622b37366ccc97?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`9c625e0`)](https://app.codecov.io/gh/apache/pinot/commit/9c625e0b9f084b2d0082ca78a9bc54f2c0337c46?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 919 commits behind head on master.
| [Files](https://app.codecov.io/gh/apache/pinot/pull/13867?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ller/api/resources/PinotSchemaRestletResource.java](https://app.codecov.io/gh/apache/pinot/pull/13867?src=pr&el=tree&filepath=pinot-controller%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcontroller%2Fapi%2Fresources%2FPinotSchemaRestletResource.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2NoZW1hUmVzdGxldFJlc291cmNlLmphdmE=) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...oller/api/resources/PinotTableRestletResource.java](https://app.codecov.io/gh/apache/pinot/pull/13867?src=pr&el=tree&filepath=pinot-controller%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcontroller%2Fapi%2Fresources%2FPinotTableRestletResource.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/13867?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (59551e4) and HEAD (9c625e0). Click for more details.
>
> <details><summary>HEAD has 38 uploads less than BASE</summary>
>
>| Flag | BASE (59551e4) | HEAD (9c625e0) |
>|------|------|------|
>|integration|7|4|
>|temurin|12|4|
>|java-21|7|3|
>|skip-bytebuffers-true|3|2|
>|skip-bytebuffers-false|7|2|
>|unittests|5|0|
>|unittests1|2|0|
>|java-11|5|1|
>|unittests2|3|0|
>|integration1|2|1|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #13867 +/- ##
=============================================
- Coverage 61.75% 0.00% -61.75%
+ Complexity 207 6 -201
=============================================
Files 2436 2502 +66
Lines 133233 138577 +5344
Branches 20636 21540 +904
=============================================
- Hits 82274 6 -82268
- Misses 44911 138571 +93660
+ Partials 6048 0 -6048
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/13867/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/13867/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/13867/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/13867/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/13867/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/13867/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%> (-61.71%)` | :arrow_down: |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/13867/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%> (-61.63%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/13867/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%> (-61.75%)` | :arrow_down: |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/13867/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%> (-27.73%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/13867/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%> (-61.75%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/13867/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/13867/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/13867/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/13867?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?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
Re: [PR] Log table config and schema updates [pinot]
Posted by "yashmayya (via GitHub)" <gi...@apache.org>.
yashmayya commented on PR #13867:
URL: https://github.com/apache/pinot/pull/13867#issuecomment-2302159646
I've updated the log lines to be uniform but I'm a little skeptical about the table deletion log. I'm worried that users will be misled into believing that the deletion process is over whereas segments will continued to be deleted async from the servers.
--
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
Re: [PR] Log table config and schema updates [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz commented on code in PR #13867:
URL: https://github.com/apache/pinot/pull/13867#discussion_r1725115771
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java:
##########
@@ -388,7 +388,7 @@ private SuccessResponse addSchema(Schema schema, boolean override, boolean force
// Best effort notification. If controller fails at this point, no notification is given.
LOGGER.info("Notifying metadata event for adding new schema {}", schemaName);
_metadataEventNotifierFactory.create().notifyOnSchemaEvents(schema, SchemaEventType.CREATE);
-
+ LOGGER.info("Successfully created new schema: {}", schema);
Review Comment:
I know it is repetitive, but in order to make it easier to find these lines... can we have a pattern closer to the update log? something like:
```
LOGGER.info("Successfully created schema: {} with value", schemaName, schema);
```
That way we can look for the regexp `Successfully (created|updated) schema: schemaImLookingFor`
--
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
Re: [PR] Log table config and schema updates [pinot]
Posted by "gortiz (via GitHub)" <gi...@apache.org>.
gortiz merged PR #13867:
URL: https://github.com/apache/pinot/pull/13867
--
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