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 2022/11/15 19:53:15 UTC
[GitHub] [druid] churromorales opened a new pull request, #13370: Unsigned integer druid complex column
churromorales opened a new pull request, #13370:
URL: https://github.com/apache/druid/pull/13370
This adds the ability to store a unsigned integer as a complex column type. Note only on disk is the value stored as an unsigned int, when it is deserialized it is a `Long` thus aggregators will not overflow. This should help save a little space for those that rely on metric columns as counts.
Docs show how to use it, basically add the extension, then your spec would have something like this:
```
"metricsSpec": [
{
"type": "unsigned_int",
"name": "value",
"fieldName": "value"
},
```
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] gianm commented on pull request #13370: Unsigned integer druid complex column
Posted by GitBox <gi...@apache.org>.
gianm commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1344753775
I am also supportive of doing `auto` long encoding by default.
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] kfaraz commented on pull request #13370: Unsigned integer druid complex column
Posted by GitBox <gi...@apache.org>.
kfaraz commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1318036146
@churromorales , could you please share some more details on the kind of savings you see with using this column?
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #13370: Unsigned integer druid complex column
Posted by GitBox <gi...@apache.org>.
clintropolis commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1318039712
How does this compare with the 'auto' `longEncoding` you can specify on `IndexSpec`? https://druid.apache.org/docs/latest/ingestion/ingestion-spec.html#indexspec. I'll admit this isn't terribly well documented, but added in https://github.com/apache/druid/pull/3148, basically it provides table and delta encoding with bitpacking which can save a fair bit of size. https://github.com/apache/druid/pull/11004 might also have some useful stuff to reference if you're looking to do any benchmarking.
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #13370: Unsigned integer druid complex column
Posted by GitBox <gi...@apache.org>.
clintropolis commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1343475115
oops, sorry for the delay
>@clintropolis i looked at the long encoding work some more and think it handles everything for us. If we take a look at VSizeLongSerde.getSerializer() it looks like it only stores the bits it needs.
yes, it does bit-packing so should effectively achieve the same thing
>I think this PR is not necessary with this feature, but I do have a suggestion. I believe this patch has been in for a while and we should make long encoding on by default. When I tested the encoding vs unsigned int, it encoded better because it rarely stored leading 0's. What do you think about me closing this PR and having another one to have long encoding on by default in druid
I think it would be reasonable to turn on by default. I used to have some worries about the performance since the abstraction seems to cause some overhead, especially in the non-vectorized engine, at least the last time I measured this https://user-images.githubusercontent.com/1577461/42849379-d1483132-89d7-11e8-8cdd-2382690d70b6.gif as seen by this chart where the 'auto' encoded data grew at a faster rate than the 'longs' encoding (top chart is basically segment scan time from 0 to 100% selection) that i collected as part of ancient #6016 (which maybe someday I will get back to...).
But, the difference wasn't huge, and I think the vectorization improvements done as part of #11004 probably make up for this, so Im ok with switching the default.
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Unsigned integer druid complex column (druid)
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1936762618
This pull request/issue has been closed due to lack of activity. If you think that
is incorrect, or the pull request requires review, you can revive the PR at any 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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Unsigned integer druid complex column (druid)
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1888184088
This pull request has been marked as stale due to 60 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If you think
that's incorrect or this pull request should instead be reviewed, please simply
write any comment. Even if closed, you can still revive the PR at any time or
discuss it on the dev@druid.apache.org list.
Thank you for your contributions.
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [PR] Unsigned integer druid complex column (druid)
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #13370: Unsigned integer druid complex column
URL: https://github.com/apache/druid/pull/13370
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] churromorales commented on pull request #13370: Unsigned integer druid complex column
Posted by GitBox <gi...@apache.org>.
churromorales commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1329892952
@clintropolis i did test it out without any encoding and it does save space. This extension can very well be used with the long encoding feature, some small changes to the long encoder because it relies on having a block of a certain size and then figures out how many long values it can stuff in there. I could add another encoder, or better yet modify the existing one (since it works) and have it work for both. But that would require a core change, but it is very possible. I think this + encoding could add much more value, but for now I don't think I have justification to change the encoder in druid-core until I show a valid reason. Let me know what you think...or have any other thoughts.
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] churromorales commented on pull request #13370: Unsigned integer druid complex column
Posted by GitBox <gi...@apache.org>.
churromorales commented on PR #13370:
URL: https://github.com/apache/druid/pull/13370#issuecomment-1332709448
@clintropolis i looked at the long encoding work some more and think it handles everything for us. If we take a look at `VSizeLongSerde.getSerializer()` it looks like it only stores the bits it needs. I think this PR is not necessary with this feature, but I do have a suggestion. I believe this patch has been in for a while and we should make long encoding on by default. When I tested the encoding vs unsigned int, it encoded better because it rarely stored leading 0's. What do you think about me closing this PR and having another one to have long encoding on by default in druid?
--
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@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org