Add support for a configurable default segment history period for segmentMetadata queries#1726
Add support for a configurable default segment history period for segmentMetadata queries#1726jon-wei wants to merge 1 commit intoapache:masterfrom
Conversation
There was a problem hiding this comment.
You could read this in directly as a Period and skip the parsing later
There was a problem hiding this comment.
maybe the default should be 1 week to be consistent with the ClientInfoResource
jon-wei
force-pushed
the
segmentmeta
branch
3 times, most recently
from
76020bb to
402956c
Compare
There was a problem hiding this comment.
would it make sense to make this a generic configuration also used by the broker as default interval for returning dimensions and metrics?
There was a problem hiding this comment.
I believe the broker currently also hardcodes one week as well, so it would be nice to make it a common config for both to be consistent, since those two are kind of related.
There was a problem hiding this comment.
That sounds reasonable, I can change the GET /datasource/* handling to use the common configuration.
Do you think druid.query.segmentMetadata.defaultSegmentHistoryPeriod is a good path/name for the shared config?
There was a problem hiding this comment.
maybe we can shorten the name a bit? maybe druid.query.segmentMetadata.defaultHistory ?
There was a problem hiding this comment.
@xvrl name changed to druid.query.segmentMetadata.defaultHistory
jon-wei
force-pushed
the
segmentmeta
branch
2 times, most recently
from
010867d to
d908777
Compare
There was a problem hiding this comment.
needs JsonProperty annotation
There was a problem hiding this comment.
Please include a serde test too.
jon-wei
force-pushed
the
segmentmeta
branch
from
d908777 to
db598ef
Compare
There was a problem hiding this comment.
I think this will confuse guice- can the broker start up with this change?
There was a problem hiding this comment.
Ok, it can start up, but it will use DateTime.now forever and ever once it does start up. It would be better to make getCurrentTime() protected and override it in the tests.
…mentMetadata queries and GET /datasources/<datasourceName> lookups
jon-wei
force-pushed
the
segmentmeta
branch
from
db598ef to
94a38ef
Compare
5 participants
Allows segmentMetadata queries to use a default interval if none is specified.
The default interval spans a configurable time period before the end time of the most recent segment.
The length of this time period can be set using ISO8601 format in the broker configuration via:
druid.query.segmentMetadata.defaultSegmentHistoryPeriod