20190115 ruler better flags #1987
Conversation
|
Could you open this against |
|
I don't think it's reasonable to inject a breaking change to release 0.5.0 after we've had two release candidates out. |
There was a problem hiding this comment.
I think this change makes much sense to clarify the ruler ring config to the user. @jtlisi could you rebase master, fix the conflicts and run make doc, please?
I don't think it's reasonable to inject a breaking change to release 0.5.0 after we've had two release candidates out.
@bboreham Given the decision to abandon the 0.5.0, I guess we're now OK with this breaking change. Am I wrong?
|
|
||
| One option to scale the ruler is by scaling it horizontally. However, with multiple ruler instances running they will need to coordinate to determine which instance will evaluate which rule. Similar to the ingesters, the rulers establish a hash ring to divide up the responsibilities of evaluating rules. | ||
|
|
||
| ## Config |
There was a problem hiding this comment.
After rebasing and make doc you will also get the config file documentation updated. Could be worth adding a link to that doc (each root config block has an anchor, so you could link it to #ruler-config) mentioning it like To see the complete set of config option, please check out...
pkg/ruler/ruler_ring.go
Outdated
| HeartbeatTimeout time.Duration `yaml:"heartbeat_timeout,omitempty"` | ||
|
|
||
| // Instance details | ||
| InstanceID string `yaml:"instance_id"` |
There was a problem hiding this comment.
Like we did in the DistributorConfig, I would suggest to add a doc:"hidden" to all Instance* fields.
jtlisi
force-pushed
the
20190115_ruler_sharding_guide
branch
from
8c94584 to
1d02d71
Compare
|
@pracucci Thanks for the feedback. I addressed your comments and this should be good for a second look. |
pkg/ruler/ruler.go
Outdated
| @@ -61,14 +61,14 @@ type Config struct { | |||
|
|
|||
| EnableSharding bool // Enable sharding rule groups | |||
| SearchPendingFor time.Duration | |||
| LifecyclerConfig ring.LifecyclerConfig | |||
| RulerRing RingConfig | |||
There was a problem hiding this comment.
The resulting yaml field name is rulerring, which looks a bit repetitive considering we're already inside the ruler: {} config. What's your take adding a yaml:"ring"? In case you decide to keep the current name, then ruler_ring may be better (easier to read for the user).
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
Signed-off-by: Jacob Lisi <jacob.t.lisi@gmail.com>
jtlisi
force-pushed
the
20190115_ruler_sharding_guide
branch
from
a7a7a8f to
93fbe0e
Compare
What this PR does:
The current ruler config exposes a number of flags that should be set to default values. For instance the ruler should never be run with a replication factor >1. However, as things stand currently the flag defaults to 3 which makes configuring the sharded ruler quite difficult. This PR adds a Ruler RingConfig struct based on the Distributor RingConfig that simplifies the ring flags for the ruler.
Which issue(s) this PR fixes:
Fixes #1986
Checklist
CHANGELOG.mdupdated