Better groupBy error messages and docs around resource limits.#4162
Merged
fjy merged 3 commits intoapache:masterfrom Apr 13, 2017
Merged
Better groupBy error messages and docs around resource limits.#4162fjy merged 3 commits intoapache:masterfrom
fjy merged 3 commits intoapache:masterfrom
Conversation
Contributor
|
👍 |
nishantmonu51
approved these changes
Apr 12, 2017
Member
nishantmonu51
left a comment
nishantmonu51
left a comment
There was a problem hiding this comment.
👍 , left some minor doc comments
| - druid.query.groupBy.maxMergingDictionarySize: size of the on-heap dictionary used when grouping on strings, per query, | ||
| in bytes. Note that this is based on a rough estimate of the dictionary size, not the actual size. | ||
| - druid.query.groupBy.maxOnDiskStorage: amount of space on disk used for aggregation, per query, in bytes. By default, | ||
| this is 0, which means all aggregation is done in-memory. |
Member
There was a problem hiding this comment.
'in-memory' refers to on-heap or offheap memory here ?
Contributor
Author
There was a problem hiding this comment.
It's both. Clarified.
| aggregation table limit, will fail with a "Resource limit exceeded" error describing the limit that was exceeded. If | ||
| maxOnDiskStorage is greater than 0, queries that exceed the in-memory limits will start using disk for aggregation. | ||
| Queries that then also exceed maxOnDiskStorage will fail with a "Resource limit exceeded" error indicating that they | ||
| ran out of disk space. |
Member
There was a problem hiding this comment.
are both on-heap dictionary and off-heap aggregation table both flushed to disk ? can we clarify this in docs.
himanshug
reviewed
Apr 12, 2017
| private static final AggregateResult DICTIONARY_FULL = AggregateResult.failure( | ||
| "Not enough dictionary space to execute this query. Try increasing " | ||
| + "druid.query.groupBy.maxMergingDictionarySize or enable disk spilling by setting " | ||
| + "druid.query.groupBy.maxOnDiskStorage to a positive number." |
Contributor
There was a problem hiding this comment.
s/positive/larger , user might already have a positive number in there
Contributor
Author
There was a problem hiding this comment.
This error will only be seen by the user if maxOnDiskStorage is 0, so "positive" does make sense. If it's > 0 they'll see a different error, about disk space being full.
himanshug
reviewed
Apr 12, 2017
| private static final AggregateResult HASHTABLE_FULL = AggregateResult.failure( | ||
| "Not enough aggregation table space to execute this query. Try increasing " | ||
| + "druid.processing.buffer.sizeBytes or enable disk spilling by setting " | ||
| + "druid.query.groupBy.maxOnDiskStorage to a positive number." |
Contributor
|
👍 aside from minor doc comments |
Contributor
Author
|
Re-pushed with some edits. |
jon-wei
approved these changes
Apr 12, 2017
Contributor
jon-wei
left a comment
jon-wei
left a comment
There was a problem hiding this comment.
👍, had one minor comment
| In addition, groupBy v1 merges results on-heap, whereas groupBy v2 merges results off-heap. These factors mean that | ||
| memory tuning and resource limits behave differently between v1 and v2. In particular, due to this, some queries | ||
| that can complete successfully in one engine may exceed resource limits and fail with the other engine. See the | ||
| "Memory tuning and resource limits" section for more details. |
Contributor
There was a problem hiding this comment.
nit: maybe turn this into a link to that section
gianm
added a commit
to implydata/druid-public
that referenced
this pull request
Apr 13, 2017
…e#4162) * Better groupBy error messages and docs around resource limits. * Fix BufferGrouper test from datasketches. * Further clarify.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improvement on #4046, based on complaints that it was hard to understand how to tune groupBy v2 for a particular query load.