You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2020/03/26 11:58:04 UTC
[GitHub] [druid] aP0StAl opened a new pull request #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
aP0StAl opened a new pull request #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568
### Description
#### Fixed the bug with allocating a large amount of memory in the buffer
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604655177
can you send me the druid query that reproduces that ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on a change in pull request #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug commented on a change in pull request #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#discussion_r399473887
##########
File path: processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java
##########
@@ -152,6 +152,8 @@ public Object deserialize(Object object)
return DoubleMeanHolder.fromBytes(StringUtils.decodeBase64(StringUtils.toUtf8((String) object)));
} else if (object instanceof DoubleMeanHolder) {
return object;
+ } else if (object instanceof byte[]) {
Review comment:
I meant, just move the byte[] check as being the first check in if-statement chain as I am expecting that would be the case most of the time.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl opened a new pull request #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl opened a new pull request #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568
### Description
Fixed the bug with allocating a large amount of memory in the buffer
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604549897
+1 after build, thanks , good catch.
How did you get to this? this was really sneaky.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl closed pull request #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl closed pull request #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl edited a comment on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl edited a comment on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604826770
@himanshug It can be either deserialize or finalizeComputation:
> {
> "queryType": "groupBy",
> "dataSource": "test_4",
> "granularity": "all",
> "dimensions": [],
> "aggregations": [
> { "type": "doubleMean", "name": "value", "fieldName": "value"}
> ],
> "intervals": [ "2018-12-01T00:00:00.000/2018-12-02T00:00:00.000" ]
> }
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604626374
got it, so you were looking at the code and knew Long.SIZE wan't number of bytes but bits.
> In some cases, the object may turn out to be byte[] . It's right?
at that point we are never gonna get a byte array.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE
for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604826770
@himanshug It can be either deserialize or finalizeComputation:
`{
"queryType": "groupBy",
"dataSource": "test_4",
"granularity": "all",
"dimensions": [],
"aggregations": [
{ "type": "doubleMean", "name": "value", "fieldName": "value"}
],
"intervals": [ "2018-12-01T00:00:00.000/2018-12-02T00:00:00.000" ]
}`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE
for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604623813
@himanshug I found this when I was trying to figure out how to implement a module to solve [this](https://github.com/apache/druid/issues/9539) issue.
I think I found another bug:
https://github.com/apache/druid/blob/55c08e07461e773309c3070ea3891ea304b343de/processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java#L148-L171
In some cases, the object may turn out to be byte[] . It's right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl commented on a change in pull request #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl commented on a change in pull request #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#discussion_r399471444
##########
File path: processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java
##########
@@ -152,6 +152,8 @@ public Object deserialize(Object object)
return DoubleMeanHolder.fromBytes(StringUtils.decodeBase64(StringUtils.toUtf8((String) object)));
} else if (object instanceof DoubleMeanHolder) {
return object;
+ } else if (object instanceof byte[]) {
Review comment:
But DoubleMeanHolder in both places too. Maybe we will replace only String in deserialize(..)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on a change in pull request #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug commented on a change in pull request #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#discussion_r399467859
##########
File path: processing/src/main/java/org/apache/druid/query/aggregation/mean/DoubleMeanAggregatorFactory.java
##########
@@ -152,6 +152,8 @@ public Object deserialize(Object object)
return DoubleMeanHolder.fromBytes(StringUtils.decodeBase64(StringUtils.toUtf8((String) object)));
} else if (object instanceof DoubleMeanHolder) {
return object;
+ } else if (object instanceof byte[]) {
Review comment:
can we make byte[] be the firs if check in both places ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug edited a comment on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604655177
can you send me the druid query that reproduces behavior that of getting byte[] in doubleMean deserialize(..) ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE
for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604641009
Are you sure? I had an exception before I fixed a similar code (deserialize and finalizeComputation):
`} else if (object instanceof byte[]) {`
` return DoubleSumVersionFilterHolder.fromBytes((byte[]) object).getSum();`
`Unknown Exception / Unknown object type [[B] / org.apache.druid.java.util.common.IAE`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug edited a comment on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604655177
can you send me the druid query that reproduces that of getting byte[] in doubleMean deserialaize(..) ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug commented on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-605110614
ok, can you please add the byte[] type handling in deserialize(..) and finalizeComputation(..) please. thanks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE
for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
aP0StAl commented on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604624356
@jihoonson Ok. I will add a test tomorrow.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug edited a comment on issue #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug edited a comment on issue #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568#issuecomment-604655177
can you send me the druid query that reproduces that of getting byte[] in doubleMean deserialize(..) ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] himanshug merged pull request #9568: fix
MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
Posted by GitBox <gi...@apache.org>.
himanshug merged pull request #9568: fix MAX_INTERMEDIATE_SIZE for DoubleMeanHolder
URL: https://github.com/apache/druid/pull/9568
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org