rename CQ config options

[beckettsean]

Instead of

compute-runs-per-interval = 10
compute-no-more-than = "2m0s"
recompute-previous-n = 2
recompute-no-older-than = "10m0s"

I propose

incremental-queries-per-interval = 10
min-time-between-batches = "2m0s"
max-resampled-queries-per-batch = 2
max-age-of-queried-data = "10m0s"

@rkuchan @gunnaraasen @dgnorton thoughts, improvements, or suggestions?

[rkuchan]

🎉👍🎉

[dgnorton]

I don't think changing the names makes them easier to understand but don't have strong feelings about it if everyone else agrees that the change makes it easier for users. Also, @pauldix my veto breaking config change.

I think the real solution is to rethink how it computes when CQs should run. It might be simpler for users if the query language allowed per-CQ settings. I think we could eliminate two of the four settings that way. E.g.,

CREATE CONTINUOUS QUERY mycq ON mydb
   PREVIOUS 1
   PERINTERVAL 1
   BEGIN
      SELECT mean(value) INTO cpu_mean FROM cpu GROUP BY time(1m)
   END

The parser could default PREVIOUS to 1 and PERINTERVAL to 1. For the example above, the query would run once for the current and previous interval. That makes the query simpler to reason about and defaulting to 1 for both of those would reduce the chances of an unexpectedly expensive CQ. The user can easily see that 1m / 1 = 1m. I.e., the query runs once every minute.

Another example:

CREATE CONTINUOUS QUERY mycq ON mydb
   PERINTERVAL 4
   BEGIN
      SELECT mean(value) INTO cpu_mean FROM cpu GROUP BY time(1h)
   END

In this example, we've taken the default PREVIOUS 1 and just specified how many times we want this CQ to run for the current interval and previous interval. 1h / 4 = 15m...so it will run once every 15 minutes.

Even better would be running only the CQs whose intervals have changed. Then we probably wouldn't need any settings. But that's a hard problem.

[pauldix]

I agree with @dgnorton's suggestions. All of these settings should be pulled from the config file in favor of a couple of easier to understand settings tied to each CQ. This goes inline with our use of config files: they're for host settings only. Anything that is applicable in the cluster should be in the metastore (thus the config should be specified via the query language).

I think that @dgnorton's two suggested settings cover all the bases and it's much more flexible since it's on a per CQ basis. What do you guys think?

[gunnaraasen]

I'm strongly in favor of @dgnorton's suggestion to set these config options per CQ.

Would it make sense to decouple the runs per interval setting for the current and previous intervals? Say there's the following CQ.

CREATE CONTINUOUS QUERY mycq ON mydb
   PREVIOUS 3
   PERINTERVAL 60
   BEGIN
      SELECT mean(value) INTO cpu_mean FROM cpu GROUP BY time(1h)
   END

The goal would be to have the latest CQ number up to date every minute. However, this would also end up recomputing the previous periods every minute when we maybe only want it to recompute once or four times. Not sure if that's a real issue people will run into or even an issue that's worth solving.

Also, if we're going to move CQ scheduling out of the config, I think it makes sense to make the continuous querier service RunInterval configurable. RunInterval is currently hard-coded to one second, which is unnecessarily high for some low power/perf machines.

[jsternberg]

I have laid out the initial ground work for allowing the scanner and parser to understand the currently proposed syntax and pushed it to the js-5136-per-cq-options branch.

I did run into one small issue with the use of PREVIOUS. The ident string "previous" is already used in one place for the fill(previous) function. When PREVIOUS gets turned into a reserved keyword, this breaks fill(previous). I was able to work around the issue by teaching the ParseExpr function to treat the PREVIOUS token in an expression context differently than in other contexts, but that may be too hacky of a solution. Is there a better name we can use?

[janantala]

It would be great to have these settings per CQ, like @dgnorton 's suggestion 👍

[jsternberg]

I've been thinking about what the name of the token should be. I don't think the word PREVIOUS helps describe what the option does. To my understanding, the option is supposed to change the number of previous intervals to resample. Maybe we should consider using RESAMPLE instead? The word PREVIOUS can refer to anything related to previous results, while RESAMPLE seems to be more specific to me.

@dgnorton @pauldix any thoughts on what word we should use in the syntax?

[pauldix]

RESAMPLE works for me

[dgnorton]

RESAMPLE isn't clear to me. It doesn't necessarily mean we're going into the past and (to me) almost sounds like a better replacement for PERINTERVAL. How about...

INTERVALS
PREVIOUS INTERVALS
RESAMPLE INTERVALS
PAST INTERVALS
PAST
PAST GROUPS
GROUPS
?

[pauldix]

Yeah, now that you mention it, RESAMPLE works better for the current interval.

Having a two word phrase feels clunky to me too. Although I don't have any better suggestions.

[gunnaraasen]

How about changing the options from integers to durations? Then the syntax could be something like RESAMPLE EVERY 10m FOR 2h.