New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add docs on storage engine WAL failover #18511
Conversation
Files changed:
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify site configuration. |
acee6e1
to
0ac8ab3
Compare
Hi folks, I added each of you to review for the following reasons/areas, but please feel free to comment on anything you see that is missing/incorrect/etc as well:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question for @jbowens re: whether we should advertise the async buffering option for file sinks in our documentation.
cc @kevin-v-ngo as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:lgtm:!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor edit suggestions -- notably a few more places I think we should mention that feature is in PREVIEW
I think we should also open a separate PR for how to monitor for failover -- the metrics to watch, how to inspect them, etc. -- thoughts?
Updated in the places where you mentioned. Do we also want to add this to the list of Features in Preview for v24.1? My assumption is yes but figured I'd ask while you're here
That makes sense, I can make a followup PR - @jbowens I found the following metrics on the custom chart debug page of a 24.1 RC cluster. Which ones do you think make sense to monitor and what values should one alert on? Based on looking I'd guess
|
Yeah, I think it makes sense to document those first three metrics:
The |
Thanks @jbowens - I've filed https://cockroachlabs.atlassian.net/browse/DOC-10268 and will do that as a followup after we get this PR in @mwang1026 are you good with this given the recent updates based on your feedback? Remaining open question is if you also want a blurb in https://www.cockroachlabs.com/docs/v24.1/cockroachdb-feature-availability.html#features-in-preview or if you'd rather this feature did not show up there. I believe our practice is to also list it there but maybe you don't want it there, idk |
@mwang1026 I went ahead and added WAL failover to the list of preview features since AFAICT we do that for everything else in Preview Let me know if you're good with the other changes and I'll send this along for docs team review so I can get it merged ASAP Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Fine to add to list of Preview features. I couldn't unsee that there are some features on that list that are GA in 24.1 but let's let those docs roll in :)
@florence-crl this is RFAL from a docs POV now in terms of sequencing, this should go in first, then #18548 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing this up! Let me know if you have questions on my suggestions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
header not aligned properly
@florence-crl thanks for the helpful review. I've incorporated everything from your first pass AFAICT - PTAL! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes: - DOC-9709 - DOC-9916 - DOC-9925 - DOC-10149 Summary of changes: - Add a new section to `cockroach start` describing the WAL failover feature, how to enable/disable, and the related logging config changes that are needed if you enable the feature - Add a new section to 'Monitoring and Alerting' docs describing the store status endpoint at `_status/stores` - Update logging docs to add some anchor links so we can refer to specific config settings from the WAL failover docs - Update v24.1 alpha release notes to link to the WAL failover docs
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
Co-authored-by: Florence Morris <florence@cockroachlabs.com>
c466735
to
094dd7f
Compare
Fixes:
Summary of changes:
Add a new section to
cockroach start
describing the WAL failover feature, how to enable/disable, and the related logging config changes that are needed if you enable the featureAdd a new section to 'Monitoring and Alerting' docs describing the store status endpoint at
_status/stores
Update logging docs to add some anchor links so we can refer to specific config settings from the WAL failover docs
Update v24.1 alpha release notes to link to the WAL failover docs