Skip to content

Doc updates; tablet throttler: check-self#686

Merged
shlomi-noach merged 2 commits intoprodfrom
tablet-throttler-check-self
Jan 28, 2021
Merged

Doc updates; tablet throttler: check-self#686
shlomi-noach merged 2 commits intoprodfrom
tablet-throttler-check-self

Conversation

@shlomi-noach
Copy link
Copy Markdown
Contributor

Documenting the changes in vitessio/vitess#7319: a new API endpoint /throttler/check-self and what it means.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
@shlomi-noach shlomi-noach requested a review from a team January 24, 2021 17:32
@netlify
Copy link
Copy Markdown

netlify bot commented Jan 24, 2021

Deploy preview for vitess ready!

Built with commit d72ff6a

https://deploy-preview-686--vitess.netlify.app

@shlomi-noach
Copy link
Copy Markdown
Contributor Author

@deepthi
Copy link
Copy Markdown
Collaborator

deepthi commented Jan 25, 2021

Let us hold this until 9.0 is out?

@shlomi-noach
Copy link
Copy Markdown
Contributor Author

Let us hold this until 9.0 is out?

Sure!

@shlomi-noach
Copy link
Copy Markdown
Contributor Author

This should be merged now, as the code has been merged.

Copy link
Copy Markdown
Collaborator

@deepthi deepthi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of suggestions. You can go ahead and merge after addressing them.


### Self health

Each tablet to its own, runs a local health check against its backend database, again in the form of evaluating replication lag from `_vt.heartbeat`. Intervals are identical to the cluster health interval illustrated above.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Each tablet to its own, runs a local health check against its backend database, again in the form of evaluating replication lag from `_vt.heartbeat`. Intervals are identical to the cluster health interval illustrated above.
Each tablet runs a local health check against its backend database, again in the form of evaluating replication lag from `_vt.heartbeat`. Intervals are identical to the cluster health interval illustrated above.


- The throttler is currently disabled by default. Use the `vttablet` option `-enable-lag-throttler` to enable the throttler.
When the throttler is disabled, it still serves `/throttler/check` API and responds with `HTTP 200 OK` to all requests.
- The throttler is currently **disabled** by default. Use the `vttablet` option `-enable-lag-throttler` to enable the throttler.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep the flags consistent and always use underscores? we have enable-lag-throttler but throttle_threshold.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right! So this is all over the place, we have dashes and underscored spread across many flags. I'm just wondering how do we go about this with existing flags? Let me see if there's something in golang that treats both as the same, as it it's an uppercase/lowercase thing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that this is not a documentation issue -- the flag is named -enable-lag-throttler and we should normalize the flag name in code.

Copy link
Copy Markdown
Contributor Author

@shlomi-noach shlomi-noach Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of digging brought up this. So what I'll do is for some variables support both dash-based and underscore-based names.

Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants