You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "clintropolis (via GitHub)" <gi...@apache.org> on 2024/10/22 01:09:17 UTC
[PR] projections fixes (druid)
clintropolis opened a new pull request, #17388:
URL: https://github.com/apache/druid/pull/17388
changes:
* fix issue when merging projections from multiple-incremental persists which was hoping that some 'dim conversion' buffers were not closed, but they already were (by the merging iterator). fix involves recreating the merging iterator to rebuild the dim conversions, which is sad, but maybe better than keeping the buffers open for the entire lifetime of the segment writeout
* fix queryable index projection to not put the time-like column as a dimension, instead only adding it as __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] projections fixes (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #17388:
URL: https://github.com/apache/druid/pull/17388#discussion_r1821282113
##########
processing/src/main/java/org/apache/druid/segment/StringDimensionMergerV9.java:
##########
@@ -176,10 +176,34 @@ public void attachParent(DimensionMergerV9 parent, List<IndexableAdapter> projec
);
StringDimensionMergerV9 stringParent = (StringDimensionMergerV9) parent;
dictionarySize = stringParent.dictionarySize;
- dimConversions = stringParent.dimConversions;
+ adapters = projectionAdapters;
+ if (projectionAdapters.size() > 1) {
+ dimConversions = Lists.newArrayListWithCapacity(projectionAdapters.size());
+ for (int i = 0; i < projectionAdapters.size(); ++i) {
+ dimConversions.add(null);
+ }
+ InitializedDictionaryMergerLookups<String> lookups = initializeDictionaryMergerLookups(projectionAdapters);
+ dictionaryMergeIterator = new DictionaryMergingIterator<>(
+ lookups.getDimValueLookups(),
+ getDictionaryMergingComparator(),
+ true
+ );
+ // iterate through merger to build dim conversions.. this is gross, but the alternative is holding open dim
Review Comment:
would it make sense to write the conversions to a tmp file rather than holding buffers open or recomputing them?
--
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] projections fixes (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis closed pull request #17388: projections fixes
URL: https://github.com/apache/druid/pull/17388
--
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] projections fixes (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on PR #17388:
URL: https://github.com/apache/druid/pull/17388#issuecomment-2471502152
closing in favor of #17460
--
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] projections fixes (druid)
Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis commented on code in PR #17388:
URL: https://github.com/apache/druid/pull/17388#discussion_r1821368221
##########
processing/src/main/java/org/apache/druid/segment/StringDimensionMergerV9.java:
##########
@@ -176,10 +176,34 @@ public void attachParent(DimensionMergerV9 parent, List<IndexableAdapter> projec
);
StringDimensionMergerV9 stringParent = (StringDimensionMergerV9) parent;
dictionarySize = stringParent.dictionarySize;
- dimConversions = stringParent.dimConversions;
+ adapters = projectionAdapters;
+ if (projectionAdapters.size() > 1) {
+ dimConversions = Lists.newArrayListWithCapacity(projectionAdapters.size());
+ for (int i = 0; i < projectionAdapters.size(); ++i) {
+ dimConversions.add(null);
+ }
+ InitializedDictionaryMergerLookups<String> lookups = initializeDictionaryMergerLookups(projectionAdapters);
+ dictionaryMergeIterator = new DictionaryMergingIterator<>(
+ lookups.getDimValueLookups(),
+ getDictionaryMergingComparator(),
+ true
+ );
+ // iterate through merger to build dim conversions.. this is gross, but the alternative is holding open dim
Review Comment:
ah yea, that makes sense, will explore that, already do something similar for the value dictionaries for nested columns. Do you think we should only do this if a column is used as a parent or would it be ok to just do always?
--
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] projections fixes (druid)
Posted by "gianm (via GitHub)" <gi...@apache.org>.
gianm commented on code in PR #17388:
URL: https://github.com/apache/druid/pull/17388#discussion_r1821378531
##########
processing/src/main/java/org/apache/druid/segment/StringDimensionMergerV9.java:
##########
@@ -176,10 +176,34 @@ public void attachParent(DimensionMergerV9 parent, List<IndexableAdapter> projec
);
StringDimensionMergerV9 stringParent = (StringDimensionMergerV9) parent;
dictionarySize = stringParent.dictionarySize;
- dimConversions = stringParent.dimConversions;
+ adapters = projectionAdapters;
+ if (projectionAdapters.size() > 1) {
+ dimConversions = Lists.newArrayListWithCapacity(projectionAdapters.size());
+ for (int i = 0; i < projectionAdapters.size(); ++i) {
+ dimConversions.add(null);
+ }
+ InitializedDictionaryMergerLookups<String> lookups = initializeDictionaryMergerLookups(projectionAdapters);
+ dictionaryMergeIterator = new DictionaryMergingIterator<>(
+ lookups.getDimValueLookups(),
+ getDictionaryMergingComparator(),
+ true
+ );
+ // iterate through merger to build dim conversions.. this is gross, but the alternative is holding open dim
Review Comment:
Probably only if it's used as a parent? Just to avoid using disk needlessly otherwise.
--
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