paul-rogers commented on code in PR #12774: URL: https://github.com/apache/druid/pull/12774#discussion_r936175722
########## docs/ingestion/compaction.md: ########## @@ -118,14 +118,14 @@ To perform a manual compaction, you submit a compaction task. Compaction tasks m |Field|Description|Required| |-----|-----------|--------| -|`type`|Task type. Should be `compact`|Yes| -|`id`|Task id|No| -|`dataSource`|Data source name to compact|Yes| +|`type`|Task type. Set the value to `compact`.|Yes| +|`id`|Task ID.|No| +|`dataSource`|Data source name to compact.|Yes| |`ioConfig`|I/O configuration for compaction task. See [Compaction I/O configuration](#compaction-io-configuration) for details.|Yes| |`dimensionsSpec`|Custom `dimensionsSpec`. The compaction task uses the specified `dimensionsSpec` if it exists instead of generating one. See [Compaction dimensionsSpec](#compaction-dimensions-spec) for details.|No| |`transformSpec`|Custom `transformSpec`. The compaction task uses the specified `transformSpec` rather than using `null`. See [Compaction transformSpec](#compaction-transform-spec) for details.|No| |`metricsSpec`|Custom `metricsSpec`. The compaction task uses the specified `metricsSpec` rather than generating one.|No| -|`segmentGranularity`|When set, the compaction task changes the segment granularity for the given interval. Deprecated. Use `granularitySpec`. |No| +|`segmentGranularity`|When set, the compaction task changes the segment granularity for the given interval. Deprecated. Use `granularitySpec`. |No| Review Comment: Suggestion: > If null (default), the new segments have the same segment granularity as the old segments. Set to specify a different segment granularity. Deprecated. Use `granularitySpec`. Or, since it is deprecated, omit the explanation: > Deprecated. Use `granularitySpec`. ########## docs/ingestion/compaction.md: ########## @@ -118,14 +118,14 @@ To perform a manual compaction, you submit a compaction task. Compaction tasks m |Field|Description|Required| |-----|-----------|--------| -|`type`|Task type. Should be `compact`|Yes| -|`id`|Task id|No| -|`dataSource`|Data source name to compact|Yes| +|`type`|Task type. Set the value to `compact`.|Yes| +|`id`|Task ID.|No| +|`dataSource`|Data source name to compact.|Yes| Review Comment: Nit: probably no need for a period in a table like this. I've gone back and forth. In the end, I don't think they add anything. Maybe change "Set the value to" to the shorter "Must be", or just show the literal value. ########## docs/ingestion/compaction.md: ########## @@ -217,7 +217,7 @@ Druid supports two supported `inputSpec` formats: |-----|-----------|--------| |`segmentGranularity`|Time chunking period for the segment granularity. Defaults to 'null', which preserves the original segment granularity. Accepts all [Query granularity](../querying/granularities.md) values.|No| |`queryGranularity`|The resolution of timestamp storage within each segment. Defaults to 'null', which preserves the original query granularity. Accepts all [Query granularity](../querying/granularities.md) values.|No| -|`rollup`|Whether to enable ingestion-time rollup or not. Defaults to 'null', which preserves the original setting. Note that once data is rollup, individual records can no longer be recovered. |No| +|`rollup`|Set the value to `true` to enable compaction-time rollup. Once the data is rolled up, you can no longer recover individual records. Defaults to 'null', which preserves the original setting.|No| Review Comment: This one requires more explanation! If the input segments are already aggregated, then it make sense that the outputs also be aggregated. If the input segments are aggregated, it is hard to see why we'd *not* want to continue to aggregate, or what happens to the input partial aggregations. Do I have to provide reduce functions to convert the aggregates to simple values? Then, if the input is detail, and the output is rolled-up, I have to provide dimensions and measures (aggregates), else, I would imagine, compaction has nothing to work with. I would suggest we a) strongly encourage the default (keep input setting), and b) create an entire section to explain what happens if the user transitions from rollup --> detail or detail --> rollup. Without that, the meaning of this setting is very unclear! -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
