-
Notifications
You must be signed in to change notification settings - Fork 469
Add docs for check external connection #19642
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
base: main
Are you sure you want to change the base?
Conversation
✅ 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 project configuration. |
c6c4daf
to
eb4a848
Compare
cc @alicia-l2 |
Note, the updated SQL diagram will pull through in an upcoming release. |
@kev-cao Just a friendly ping to review this, thanks! |
src/current/_includes/v25.1/backups/external-storage-check-tip.md
Outdated
Show resolved
Hide resolved
@kev-cao Fixed, 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.
one small comment - otherwise LGTM! thank you!
@kev-cao can you PTAL when you're ready, 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, small non-blocking comments
|
||
Option | Value | Description | ||
--------+-------+------------ | ||
`concurrently` | `INT` | Run multiple connection tests concurrently. If you also set the `time` option, it will run the specified number of concurrent tests until the time has elapsed. By default, only `1` connection test will run. |
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.
i assume concurrentLY
is intentional and is not concurrenCY
?
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.
Yep, it's concurrently
!
Option | Value | Description | ||
--------+-------+------------ | ||
`concurrently` | `INT` | Run multiple connection tests concurrently. If you also set the `time` option, it will run the specified number of concurrent tests until the time has elapsed. By default, only `1` connection test will run. | ||
`time` | `STRING` | Run the test repeatedly until the duration has elapsed. |
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.
if this is an actual DURATION
it would be good to link to those docs
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.
It's a duration string, which we don't have type docs for
--------+-------+------------ | ||
`concurrently` | `INT` | Run multiple connection tests concurrently. If you also set the `time` option, it will run the specified number of concurrent tests until the time has elapsed. By default, only `1` connection test will run. | ||
`time` | `STRING` | Run the test repeatedly until the duration has elapsed. | ||
`transfer` | `STRING` | The size of the file that is written and read during each iteration of the connection test. By default, this will transfer a `32MiB` file. |
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.
was curious if we have docs somewhere on how we format file sizes in the STRING
data type, e.g. what if i type 32MB
instead of 32MiB
- i just looked and we don't, but we do pass values like this as strings in a bunch of places so it's hopefully obvious
------|-------|------------ | ||
`node` | `INT` | The node ID. | ||
`locality` | `STRING` | The [locality]({% link {{ page.version.version }}/cockroach-start.md %}#locality) of the node. | ||
`ok` | `BOOL` | The success of the test run. |
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.
small but i would find this clearer as "whether the test run succeeded"
`transferred` | `STRING` | The size of the file transferred during the test. | ||
`read_speed` | `STRING` | The speed at which the node read the test file. | ||
`write_speed` | `STRING` | The speed at which the node wrote the test file. | ||
`can_delete` | `BOOL` | The success of file deletion. |
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.
suggest "whether file deletion succeeded" (if accurate)
`write_speed` | `STRING` | The speed at which the node wrote the test file. | ||
`can_delete` | `BOOL` | The success of file deletion. | ||
|
||
## Examples |
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.
nit: singular 'example'
OR
could create a subheading 'Test an external connection', which would leave room for adding examples in future if desired
i slightly lean latter but either would work i think
--- | ||
title: CHECK EXTERNAL CONNECTION | ||
summary: Test the connection of each node to your cloud storage location. | ||
toc: true |
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.
Suggest adding docs_area: reference.sql
to the frontmatter so this matches our other pages
(PS i should probably go look if this still matters? ISTR Jesse was doing this for "reasons" but i no longer remember why)
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.
I didn't think we had to use this anymore, and checked in with Mike and he confirmed it's ok to drop that in the FM.
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, I filed https://cockroachlabs.atlassian.net/browse/DOC-13935 for us to remove them
682d7ca
to
f714b9c
Compare
Fixes DOC-10392
This PR adds SQL syntax page for the
CHECK EXTERNAL CONNECTION
statement, which tests the connection from each node to a provided URI.Also, adds a tip to check/use the tool on relevant pages.
Added to v25.1 and v25.2 docs.
Preview
https://deploy-preview-19642--cockroachdb-docs.netlify.app/docs/v25.2/check-external-connection.html