[DOCS]Add info on calendar v fixed interval. Closes #29410 #31638
Conversation
Sue-Gallagher
requested review from
pcsanwald,
colings86,
debadair and
DaveCTurner
There was a problem hiding this comment.
@Sue-Gallagher I left some comments but I think this is an great improvement on what we have today, thanks for taking this on
| that time based intervals are not fixed (think of leap years and on the number of days in a month). For this reason, | ||
| we need special support for time based data. From a functionality perspective, this histogram supports the same features | ||
| as the normal <<search-aggregations-bucket-histogram-aggregation,histogram>>. The main difference is that the interval can be specified by date/time expressions. | ||
| This multi-bucket aggregation supports exactly the same features as the normal |
There was a problem hiding this comment.
I think I prefer the original wording of saying its similar to the histogram aggregation as we may diverge features from the histogram aggregation if it makes sense
| are working with a _calendar interval_, but multiples, such as 6 hours or 3 days, are | ||
| _fixed-duration intervals_. | ||
|
|
||
| For example, a specification of 1 day (1d) is a calendar interval that means "at |
There was a problem hiding this comment.
Can we instead say For example, a specification of 1 day (1d) from now is a calendar interval that means...?
| This multi-bucket aggregation supports exactly the same features as the normal | ||
| <<search-aggregations-bucket-histogram-aggregation,histogram>>, but it can | ||
| only be applied on date values. Because dates are represented internally in | ||
| Elasticsearch as long values, it is possible, but not as accurate, to use the normal `histogram` on dates as well. The main difference in the two APIs is |
There was a problem hiding this comment.
We should correct the wrapping here
| the specified interval includes the change to or from daylight savings time, the | ||
| interval will end an hour sooner or later than you expect. | ||
|
|
||
| There are similar differences to consider when you specify single versus multiple minutes or hours. Multiple time periods longer than a day are not supported. |
There was a problem hiding this comment.
We should correct the wrapping here
| * Multiple days (_n_d) are intervals of exactly 24x60x60x1000=86,400,000 milliseconds each. | ||
|
|
||
| *weeks (w)* + | ||
| * One week (1w) is the interval between the start day:hour:minute:second and |
There was a problem hiding this comment.
Can we clarify this be changing day to day_of_week since day could mean "day of week" of "day of month" or, more rarely, "day of year"?
| All hours begin at 00 minutes and 00 seconds. + | ||
| * One hour (1h) is the interval between 00:00 minutes of the first hour and 00:00 | ||
| minutes of the following hour in the specified timezone, compensating for any | ||
| intervening leap seconds, so that the number of minutes and seconds past the hour is the same at the start and end. |
There was a problem hiding this comment.
We should correct the wrapping here
| * Multiple weeks (_n_w) are not supported. | ||
|
|
||
| *months (m)* + | ||
| * One month (1m) is the interval between the start date and time and the same date and time of the following month in the specified timezone, so that the date and time are the same at the start and end. |
There was a problem hiding this comment.
start date and time I think might be confusing here. What we really mean is start day of month and time and same day of month and time but that language probably needs a little tweaking. Unfortunately start date is a little ambiguous to me here.
We should also correct the wrapping here
|
|
||
| *years (y)* + | ||
| * One year (1y) is the interval between the start month, date, and time and the same | ||
| month, date, and time the following year in the specified timezone, so that the date and time are the same at the start and end. |
There was a problem hiding this comment.
Again I think we need to use "day of month" here to be unambiguous.
We should also correct the wrapping here
| * Multiple years (_n_y) are not supported. | ||
|
|
||
| *quarters (q)* + | ||
| * One quarter (1q) is the interval between the start month, date, and time and the same date and time three months later, so that the date and time are the |
There was a problem hiding this comment.
Again I think we need to use "day of month" here to be unambiguous.
We should also correct the wrapping here.
Also can we move this above the years section? its a small thing but I like it ordered by increasing intervals 😄
colings86
added
>docs
|
Pinging @elastic/es-search-aggs |
|
|
||
| Here are the valid time specifications and their meanings: | ||
|
|
||
| *milliseconds (ms)* + |
There was a problem hiding this comment.
I built the docs locally and tested, and this section seems formatted strangely, I've attached a screenshot. the * characters I think are intended as bold, but this isn't happening for some reason
There was a problem hiding this comment.
That's the last time I believe the asciidocs user guide. Fixed:
There was a problem hiding this comment.
@Sue-Gallagher I don't see any new commits on this PR that show the fix your comment is referencing. is it possible you haven't pushed from your local machine back to the remote?
There was a problem hiding this comment.
Should be there now and there'll be at least one more to address Dave T's comments
| Widely distributed applications must also consider vagaries such as countries that | ||
| start and stop daylight savings time at 12:01 A.M., so end up with one minute of | ||
| Sunday followed by an additional 59 minutes of Saturday every fall, and countries | ||
| that decide at whim to move across the international date line. Situations like |
There was a problem hiding this comment.
my personal opinion is that this sentence is just as impactful if we remove the phrase "at whim".
| of the following day in the specified timezone, compensating for any intervening | ||
| time changes, so that the number of hours, minutes, and seconds of the day is | ||
| the same at the start and end. | ||
| * Multiple days (_n_d) are intervals of exactly 24x60x60x1000=86,400,000 milliseconds each. |
There was a problem hiding this comment.
I've never seen the _n_d parameter before, and I grepped the elasticsearch codebase for it, and I haven't found any other occurrences of it. this notation is confusing to me, and I'm not sure what it refers to.
There was a problem hiding this comment.
Another format glitch - the n should be italicized. Fixed.
|
|
||
| Widely distributed applications must also consider vagaries such as countries that | ||
| start and stop daylight savings time at 12:01 A.M., so end up with one minute of | ||
| Sunday followed by an additional 59 minutes of Saturday every fall, and countries |
There was a problem hiding this comment.
nit: I think "once per year" is clearer than "every fall", as "fall" is one of those words that can be confusing. it's common in the US, but not so sure about elsewhere.
There was a problem hiding this comment.
I think we should drop this specific example (it occurred only 24 times and only in a handful of places in eastern Canada) and instead recommend more generally something like:
We recommend careful testing of the behaviour of your application with data in the vicinity of daylight saving time transitions, and other timezone-changing events, to ensure that it operates as desired.
|
|
||
| Internally, a date is represented as a 64 bit number representing a timestamp | ||
| in milliseconds-since-the-epoch. These timestamps are returned as the bucket | ||
| in milliseconds-since-the-epoch (01/01/1970). These timestamps are returned as | ||
| the bucket |
There was a problem hiding this comment.
"as the key name of the bucket" is clearer, IMO.
| the bucket covering that day will only hold data for 23 hours instead of the usual | ||
| 24 hours for other buckets. The same is true for shorter intervals like e.g. 12h. | ||
| Here, we will have only a 11h bucket on the morning of 27 March when the DST shift | ||
| happens. |
There was a problem hiding this comment.
why would we remove this?
There was a problem hiding this comment.
+1 I think this is important.
There was a problem hiding this comment.
Redundant to:
For example, a specification of 1 day (1d) from now is a calendar interval that
means "at
this exact time tomorrow" no matter the length of the day. A change to or from
daylight savings time that results in a 23 or 25 hour day is compensated for and the
specification of "this exact time tomorrow" is maintained.
"Setting intervals" section, paragraph 3.
Colin approved deletion.
There was a problem hiding this comment.
I'm still 👎 on deletion of this. The section on intervals covers similar ground, but I think (a) it is worth repeating, (b) here we spell out explicitly that the buckets are different sizes, and (c) here we have an example, which I find to be almost essential in understanding how these things work. @colings86 are you sure about losing this?
There was a problem hiding this comment.
I also think we should keep this paragraph, for the reasons that @DaveCTurner outlines
| full hour. While these creations help keep us in sync with the cosmos and our | ||
| environment, they can make specifying time intervals accurately a real challenge. | ||
| The only universal truth our researchers have yet to disprove is that a | ||
| millisecond is always the same duration, and a second is always 1000 milliseconds. |
There was a problem hiding this comment.
Specified below.
| means "at | ||
| this exact time tomorrow" no matter the length of the day. A change to or from | ||
| daylight savings time that results in a 23 or 25 hour day is compensated for and the | ||
| specification of "this exact time tomorrow" is maintained. But if you specify 2 or |
There was a problem hiding this comment.
Careful - sometimes "this exact time tomorrow" doesn't exist. However, this is overly general: buckets specified in days start at the start of the day in question, which is normally midnight (the first such if there are multiple midnights) but if there is no midnight then it's the earliest time that occurs on the day in question.
There was a problem hiding this comment.
added:
NOTE:
In all cases, when the specified end time does not exist, the actual end time is the
closest available time after the specified end.
| minute that contains a leap-second, which is 2000ms long); supports multiples. | ||
|
|
||
| *minutes (m)* + | ||
| All minutes begin at 00 seconds. + |
There was a problem hiding this comment.
I checked this and it seems correct: e.g. Africa/Monrovia changed their clocks by +44½ minutes at 30 seconds past midnight on 1972-01-07, and the buckets so generated are labelled:
1972-01-06T23:58:00.000-00:44:30(60 seconds long)1972-01-06T23:59:00.000-00:44:30(90 seconds long, includes midnight(!))1972-01-07T00:45:00.000Z(60 seconds long)
| each. | ||
|
|
||
| *hours (h)* + | ||
| All hours begin at 00 minutes and 00 seconds. + |
There was a problem hiding this comment.
I checked this and it seems correct: e.g. Australia/Lord_Howe has a 30-minute DST offset, so e.g. in spring 2017 the buckets generated are labelled:
2017-10-01T00:00:00.000+10:30(60 minutes long)2017-10-01T01:00:00.000+10:30(90 minutes long)2017-10-01T03:00:00.000+11:00(60 minutes long)
There was a problem hiding this comment.
...whenever possible???
There was a problem hiding this comment.
nono, I think what you wrote is correct as is.
| (midnight). | ||
| * One day (1d) is the interval between 00:00:00 of the first day and 00:00:00 | ||
| of the following day in the specified timezone, compensating for any intervening | ||
| time changes, so that the number of hours, minutes, and seconds of the day is |
There was a problem hiding this comment.
I think this isn't true. As far as I can tell there are no consecutive days containing time zone transitions, so if one day contains no midnight then the next day certainly contains exactly one, and the bucket for the no-midnight day will end at the midnight of the following day.
There was a problem hiding this comment.
I'm not sure where you got "consecutive days containing timezone transitions", and Colin approved this. If I change "time changes" to "time change", will that sound better to you?
There was a problem hiding this comment.
No, the issue is with days that contain no midnight. For instance, São Paolo set their clocks forwards from -3:00 to -2:00 at 0300 UTC on 2017-10-15, so in local time 2017-10-14 23:59:59 is followed by 2017-10-15 01:00:00. In this case the nearby buckets that we generate are:
2017-10-14T00:00:00.000-03:00(24 hours long)2017-10-15T01:00:00.000-02:00(23 hours long)2017-10-16T00:00:00.000-02:00(24 hours long)
Thus the day is not always the interval between midnight of the first day and midnight of the second, and the hours of the start and end time of the bucket are not always equal. However, at least one of the start or the end is at midnight, because there's never two consecutive days without a midnight.
There was a problem hiding this comment.
I re-read my original comment and it does seems rather unclear what I actually meant. Sorry about that - hopefully this helps.
|
|
||
| WARNING: | ||
| To avoid unexpected results, all connected servers and clients must sync to the | ||
| same network time service. |
There was a problem hiding this comment.
I think we should leave out the "same network time service" bit. Syncing to any (non-broken) time service is going to give you reasonably accurate clock sync. For instance, by default, Windows machines use a different default time service from Linux machines, and this is fine. How about:
To avoid unexpected results, ensure that the clocks on all servers and clients are well-synchronized using a network time service or similar functionality.
There was a problem hiding this comment.
See issue #29753
There was a problem hiding this comment.
Sure, that doesn't mean it's right to say "the same network time service". Syncing to any (non-broken) time service would do, as they are all sufficiently in sync.
There was a problem hiding this comment.
Changed "the same" to "a reliable"
|
|
||
| Internally, a date is represented as a 64 bit number representing a timestamp | ||
| in milliseconds-since-the-epoch. These timestamps are returned as the bucket | ||
| in milliseconds-since-the-epoch (01/01/1970). These timestamps are returned as |
There was a problem hiding this comment.
I think we should clarify that the epoch is UTC midnight on January 1 1970.
|
|
||
| Time zones may either be specified as an ISO 8601 UTC offset (e.g. `+01:00` or | ||
| You can specify timezones as either an ISO 8601 UTC offset (e.g. `+01:00` or | ||
| `-08:00`) or as a timezone id, an identifier used in the TZ database like |
There was a problem hiding this comment.
Can we call this the IANA timezone database? There are other timezone databases with their own identification schemes (notably Windows has its own).
| @@ -174,7 +276,7 @@ on 1 October 2015: | |||
| --------------------------------- | |||
| // TESTRESPONSE[s/\.\.\./"took": $body.took,"timed_out": false,"_shards": $body._shards,"hits": $body.hits,/] | |||
|
|
|||
| If a `time_zone` of `-01:00` is specified, then midnight starts at one hour before | |||
| If you specify a `time_zone` of `-01:00`, then midnight starts at one hour before | |||
There was a problem hiding this comment.
"midnight starts at" is strange wording. I think "midnight is" is better.
| the bucket covering that day will only hold data for 23 hours instead of the usual | ||
| 24 hours for other buckets. The same is true for shorter intervals like e.g. 12h. | ||
| Here, we will have only a 11h bucket on the morning of 27 March when the DST shift | ||
| happens. |
There was a problem hiding this comment.
+1 I think this is important.
| each. | ||
|
|
||
| *hours (h)* + | ||
| All hours begin at 00 minutes and 00 seconds. + |
There was a problem hiding this comment.
nono, I think what you wrote is correct as is.
| (midnight). | ||
| * One day (1d) is the interval between 00:00:00 of the first day and 00:00:00 | ||
| of the following day in the specified timezone, compensating for any intervening | ||
| time changes, so that the number of hours, minutes, and seconds of the day is |
There was a problem hiding this comment.
No, the issue is with days that contain no midnight. For instance, São Paolo set their clocks forwards from -3:00 to -2:00 at 0300 UTC on 2017-10-15, so in local time 2017-10-14 23:59:59 is followed by 2017-10-15 01:00:00. In this case the nearby buckets that we generate are:
2017-10-14T00:00:00.000-03:00(24 hours long)2017-10-15T01:00:00.000-02:00(23 hours long)2017-10-16T00:00:00.000-02:00(24 hours long)
Thus the day is not always the interval between midnight of the first day and midnight of the second, and the hours of the start and end time of the bucket are not always equal. However, at least one of the start or the end is at midnight, because there's never two consecutive days without a midnight.
There was a problem hiding this comment.
My comments have been addressed so I'm approving, but I do think that @DaveCTurner 's issues should be addressed (and he should review and eventually approve) prior to merging.
There was a problem hiding this comment.
Suggested some alternatives for the no-midnight problem, and pinged @colings86 about the deleted paragraph. Otherwise everything from me has been addressed.
| means "at | ||
| this exact time tomorrow" no matter the length of the day. A change to or from | ||
| daylight savings time that results in a 23 or 25 hour day is compensated for and the | ||
| specification of "this exact time tomorrow" is maintained. But if you specify 2 or |
|
|
||
| WARNING: | ||
| To avoid unexpected results, all connected servers and clients must sync to a | ||
| reliable network time service. |
| in milliseconds-since-the-epoch. These timestamps are returned as the bucket | ||
| ++key++s. The `key_as_string` is the same timestamp converted to a formatted | ||
| date string using the format specified with the `format` parameter: | ||
| in milliseconds-since-the-epoch (01/01/1970 midnight UTC). These timestamps are |
| `-08:00`) or as a timezone id, an identifier used in the TZ database like | ||
| `America/Los_Angeles`. | ||
| You can specify timezones as either an ISO 8601 UTC offset (e.g. `+01:00` or | ||
| `-08:00`) or as a timezone ID as specified in the IANA timezone database, |
There was a problem hiding this comment.
IANA timezone database 👍
| the bucket covering that day will only hold data for 23 hours instead of the usual | ||
| 24 hours for other buckets. The same is true for shorter intervals like e.g. 12h. | ||
| Here, we will have only a 11h bucket on the morning of 27 March when the DST shift | ||
| happens. |
There was a problem hiding this comment.
I'm still 👎 on deletion of this. The section on intervals covers similar ground, but I think (a) it is worth repeating, (b) here we spell out explicitly that the buckets are different sizes, and (c) here we have an example, which I find to be almost essential in understanding how these things work. @colings86 are you sure about losing this?
| All days begin at the earliest possible time, which is usually 00:00:00 | ||
| (midnight). | ||
|
|
||
| * One day (1d) is the interval between 00:00:00 of the first day and 00:00:00 |
There was a problem hiding this comment.
I think we need to lose the explicit 00:00:00s in this line. We can correctly say "between the start of the day and the start of the following day", but the start is not always 00:00:00 (disclaimer below notwithstanding).
| * One day (1d) is the interval between 00:00:00 of the first day and 00:00:00 | ||
| of the following day in the specified timezone, compensating for any intervening | ||
| time changes, so that the number of hours, minutes, and seconds of the day is | ||
| the same at the start and end. |
There was a problem hiding this comment.
Similarly, I don't think we should say this as it's not true for days that don't have a midnight. Could the clause starting "so that..." just be deleted?
|
|
||
| * One day (1d) is the interval between the start of the day and the start of | ||
| of the following day in the specified timezone, compensating for any intervening | ||
| time changes, so that the number of hours, minutes, and seconds of the day is |
There was a problem hiding this comment.
This "so that..." phrase still seems to be here, although its removal was marked as "done". Maybe a missing push?
There was a problem hiding this comment.
This PR now LGTM, thanks for the extra iterations @Sue-Gallagher
|
|
||
| months (m) :: | ||
|
|
||
| * One month (1m) is the interval between the start day of the month and time of |
There was a problem hiding this comment.
I believe the unit for month is supposed to be capital M, lowercase m is minutes
|
@colings86 can you have another look ? Others have approved this, not sure there's anything left to address from your comments. |
|
@debadair, do we know the status of this PR? Would be good to get this information merged, especially since I'd like to tweak some of it when #33727 merges (since it will adjust how the user specifies these fixed/calendar times). I'm happy to take it over as well, just didn't want to grab it if there were plans or work still being done. |
|
@polyfractal would you be able to take this one over as you suggested and work towards merging it? |
Extensive edit to add additional information on the difference between calendar intervals and fixed-length intervals.
Extensive edit to add additional information on the difference between calendar intervals and fixed-length intervals.
Extensive edit to add additional information on the difference between calendar intervals and fixed-length intervals.
Extensive edit to add additional information on the difference between calendar intervals and fixed-length intervals.
Extensive edit to add additional information on the difference between calendar intervals and fixed-length intervals.