Add back RAM accounting and rate limiting #15712
Conversation
| T value = (T) expression.value(); | ||
| ramAccounting.addBytes(dataType.valueBytes(value)); | ||
| statsBuilder.add((T) expression.value()); |
There was a problem hiding this comment.
| T value = (T) expression.value(); | |
| ramAccounting.addBytes(dataType.valueBytes(value)); | |
| statsBuilder.add((T) expression.value()); | |
| T value = (T) expression.value(); | |
| ramAccounting.addBytes(dataType.valueBytes(value)); | |
| statsBuilder.add(value); |
| T value = (T) expression.value(); | ||
| ramAccounting.addBytes(dataType.valueBytes(value)); |
There was a problem hiding this comment.
Can you double check that the cast is safe without going through dataType.sanitizeValue? E.g. if the source lookup interpretes a LONG as INTEGER due to the json parser behavior?
There was a problem hiding this comment.
Do you mean replace the cast with T value = dataType.sanitizeValue(expression.value());?
There was a problem hiding this comment.
That would be the safe option but I'm not sure if it is required. (A case where you use a LONG type with a small integer like 4 should tell you)
There was a problem hiding this comment.
Gotcha.
I think this should in fact always work because practically speaking the expression and the sketch will always be parametrized with the same type, but because of the way things are constructed we lose the type information of the expression somewhere along the way. For now I'll add in the sanitizeValue call. We may be able to remove it later.
| } | ||
| accounting.addBytes(991); // still cached | ||
|
|
||
| Exception e = expectThrows(RuntimeException.class, () -> accounting.addBytes(1)); |
There was a problem hiding this comment.
Please use assertThatThrownBy
| @Override | ||
| public void addBytes(long bytes) { | ||
| total += bytes; | ||
| if (total > 32) { |
There was a problem hiding this comment.
Maybe add a comment that 32 == SHIFTED_BLOCK_SIZE
romseygeek
merged commit 420fb8f
into
feature/analyze-datasketches
Column sketches now keep track of how much memory is being used, and how fast data is being added to the sketches. Experiments show that a column sketch with three internal sketches uses between 2 and 4% of the memory of a fully realised java array of samples, so we adjust the amount of memory tracked downwards by a factor of 32.
Column sketches now keep track of how much memory is being used, and how
fast data is being added to the sketches. Experiments show that a column sketch
with three internal sketches uses between 2 and 4% of the memory of a fully
realised java array of samples, so we adjust the amount of memory tracked
downwards by a factor of 32.