refresh AWS deployment doc + relevant includes #5677
Conversation
tweaks to AWS deployment docs
|
This change is |
There was a problem hiding this comment.
I have some questions and requests, but this is really great work overall, @taroface.
Once you've iterated on the clock sync changes, I'd suggest looping Ben Darnell in to review that portion.
For the load balancing portion, I'd also confirm with Ben or Marc and, if we stick with NLB, also close that ALB issue.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @jseldess and @taroface)
_includes/v19.1/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
~~~ shell $ cockroach start \ --certs-dir=certs \
This is the include for INSECURE_scale_cluster. Changing this make the insecure tutorial wrong, for example: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/01092c62f425cb2ef65cd0736f9899b33e75e0a2/v19.1/deploy-cockroachdb-on-aws-insecure.html#step-10-scale-the-cluster
_includes/v19.1/prod-deployment/insecure-test-cluster.md, line 19 at r2 (raw file):
~~~ 3. Use `\q` or `ctrl-d` to exit the SQL shell.
I think this was intended to show that the database you create when connected on one node is available from any other node, similar to https://www.cockroachlabs.com/docs/dev/start-a-local-cluster.html#step-2-use-the-built-in-sql-client. But it's clearly not clear. Should we rephrase the step intro perhaps?
_includes/v19.1/prod-deployment/secure-generate-certificates.md, line 56 at r2 (raw file):
5. Upload the CA certificate and node certificate and key to the first node: {% if page.title contains "AWS" %}
Nice! Was this problem in a GitHub issue, or did you just discover it during testing?
_includes/v19.1/prod-deployment/secure-test-cluster.md, line 19 at r2 (raw file):
~~~ 3. Use `\q` to exit the SQL shell.
Same comment/question as above.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 87 at r2 (raw file):
Amazon provides the [Amazon Time Sync Service](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html), which uses a fleet of satellite-connected and atomic reference clocks in each AWS Region to deliver accurate current time readings. The service also smears the leap second. - If you plan to run your entire cluster on AWS, [configure each AWS instance to use the internal Amazon Time Sync Service](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html#configure-amazon-time-service).
I think we still need to call out the case where some nodes are running on AWS and other nodes are on other clouds. Based on the issue, instead of the previous guidance (use Googles time service for all nodes, even those on AWS), we can now say that it's ok to use Amazon's service for AWS nodes, and Google's service for GCP nodes.
Also, we should refactor the third bullet here as well: https://www.cockroachlabs.com/docs/stable/recommended-production-settings.html#considerations
_includes/v19.2/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
~~~ shell $ cockroach start \ --certs-dir=certs \
See earlier comment.
_includes/v19.2/prod-deployment/insecure-test-cluster.md, line 19 at r2 (raw file):
~~~ 3. Use `\q` or `ctrl-d` to exit the SQL shell.
See earlier comment.
_includes/v19.2/prod-deployment/secure-test-cluster.md, line 19 at r2 (raw file):
~~~ 3. Use `\q` to exit the SQL shell.
See earlier comment.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 88 at r2 (raw file):
- If you plan to run your entire cluster on AWS, [configure each AWS instance to use the internal Amazon Time Sync Service](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html#configure-amazon-time-service). - However, if you plan to run a hybrid cluster across AWS and other cloud providers or environments, [configure all machines to use Google's external NTP service](deploy-cockroachdb-on-digital-ocean.html#step-2-synchronize-clocks), which is comparably accurate and also handles <a href="https://developers.google.com/time/smear">"smearing" the leap second</a>.
See earlier comment.
There was a problem hiding this comment.
Reviewable status:
_includes/v19.1/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
This is the include for
INSECURE_scale_cluster. Changing this make the insecure tutorial wrong, for example: http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/01092c62f425cb2ef65cd0736f9899b33e75e0a2/v19.1/deploy-cockroachdb-on-aws-insecure.html#step-10-scale-the-cluster
Should have caught this before it went out. Removed the offending line in a new commit!
_includes/v19.1/prod-deployment/insecure-test-cluster.md, line 19 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
I think this was intended to show that the database you create when connected on one node is available from any other node, similar to https://www.cockroachlabs.com/docs/dev/start-a-local-cluster.html#step-2-use-the-built-in-sql-client. But it's clearly not clear. Should we rephrase the step intro perhaps?
Makes sense. I attempted a rephrase, which makes the intro longer. Not sure if it is clear enough yet.
Also, amended "address of a different node" to "address of the load balancer".
_includes/v19.1/prod-deployment/secure-generate-certificates.md, line 56 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Nice! Was this problem in a GitHub issue, or did you just discover it during testing?
I'm not sure if a GitHub issue exists for it, but I had to piece together the proper syntax using this doc (which I originally considered linking directly, but I thought it would add too much information noise): https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AccessingInstancesLinux.html
_includes/v19.1/prod-deployment/secure-test-cluster.md, line 19 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same comment/question as above.
Done.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 87 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
I think we still need to call out the case where some nodes are running on AWS and other nodes are on other clouds. Based on the issue, instead of the previous guidance (use Googles time service for all nodes, even those on AWS), we can now say that it's ok to use Amazon's service for AWS nodes, and Google's service for GCP nodes.
Also, we should refactor the third bullet here as well: https://www.cockroachlabs.com/docs/stable/recommended-production-settings.html#considerations
Added extra verbiage in the AWS, GCE, and Production Settings articles.
_includes/v19.2/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
See earlier comment.
Fixed in new commit.
_includes/v19.2/prod-deployment/insecure-test-cluster.md, line 19 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
See earlier comment.
Done.
_includes/v19.2/prod-deployment/secure-test-cluster.md, line 19 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
See earlier comment.
Done.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 88 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
See earlier comment.
Done.
|
Hi @bdarnell, would you be able to review my updates to the following: Regarding #4847: Regarding #5317, can you confirm that NLB is indeed the requirement for deploying on AWS, and not ALB? I have tested an AWS deployment using an NLB and have specifically called this out in the instructions: |
There was a problem hiding this comment.
Reviewable status:
_includes/v19.1/faq/clock-synchronization-effects.md, line 11 at r3 (raw file):
- We recommend using [Google Public NTP](https://developers.google.com/time/) or [Amazon Time Sync Service](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html) with the clock sync service you are already using (e.g., [`ntpd`](http://doc.ntp.org/), [`chrony`](https://chrony.tuxfamily.org/index.html)). For example, if you are already using `ntpd`, configure `ntpd` to point to the Google or Amazon time server. {{site.data.alerts.callout_info}}
We should completely remove this callout.
_includes/v19.1/faq/clock-synchronization-effects.md, line 15 at r3 (raw file):
Let's be more specific about the hybrid nature:
In a hybrid GCE/AWS cluster,...
_includes/v19.1/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Should have caught this before it went out. Removed the offending line in a new commit!
Hmm, it looks like the --insecure flag is entirely gone now? You need that.
_includes/v19.1/prod-deployment/insecure-test-cluster.md, line 19 at r2 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Makes sense. I attempted a rephrase, which makes the intro longer. Not sure if it is clear enough yet.
Also, amended "address of a different node" to "address of the load balancer".
Looks good. Thanks.
_includes/v19.1/prod-deployment/secure-generate-certificates.md, line 56 at r2 (raw file):
Previously, taroface (Ryan Kuo) wrote…
I'm not sure if a GitHub issue exists for it, but I had to piece together the proper syntax using this doc (which I originally considered linking directly, but I thought it would add too much information noise): https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/AccessingInstancesLinux.html
Good sleuthing here.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 84 at r3 (raw file):
{{site.data.alerts.callout_info}} The Google and Amazon services handle <a href="https://developers.google.com/time/smear">"smearing" the leap second</a> in compatible ways.
You can use the markdown link syntax here if you like.
["smearing" the leap second](https://developers.google.com/time/smear)
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 97 at r3 (raw file):
{{site.data.alerts.callout_info}} The Google and Amazon services handle <a href="https://developers.google.com/time/smear">"smearing" the leap second</a> in compatible ways.
Same as above.
_includes/v19.2/faq/clock-synchronization-effects.md, line 11 at r3 (raw file):
- We recommend using [Google Public NTP](https://developers.google.com/time/) or [Amazon Time Sync Service](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html) with the clock sync service you are already using (e.g., [`ntpd`](http://doc.ntp.org/), [`chrony`](https://chrony.tuxfamily.org/index.html)). For example, if you are already using `ntpd`, configure `ntpd` to point to the Google or Amazon time server. {{site.data.alerts.callout_info}}
Same as before.
_includes/v19.2/faq/clock-synchronization-effects.md, line 15 at r3 (raw file):
{{site.data.alerts.end}} - In a hybrid cluster, GCE machines should use [Google's internal NTP service](https://cloud.google.com/compute/docs/instances/managing-instances#configure_ntp_for_your_instances) and AWS machines should use [Amazon Time Sync Service](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html#configure-amazon-time-service). The Google and Amazon services handle ["smearing" the leap second](https://developers.google.com/time/smear) in compatible ways.
Same as before.
_includes/v19.2/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Fixed in new commit.
--insecure seems to be missing now.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 97 at r3 (raw file):
{{site.data.alerts.callout_info}} The Google and Amazon services handle <a href="https://developers.google.com/time/smear">"smearing" the leap second</a> in compatible ways.
Same as before.
There was a problem hiding this comment.
Reviewable status:
_includes/v19.1/faq/clock-synchronization-effects.md, line 11 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
We should completely remove this callout.
Done.
_includes/v19.1/faq/clock-synchronization-effects.md, line 15 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Let's be more specific about the hybrid nature:
In a hybrid GCE/AWS cluster,...
Done. Also added the instruction to use Google Public NTP for non-GCE/AWS machines.
_includes/v19.1/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Hmm, it looks like the
--insecureflag is entirely gone now? You need that.
Done.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 84 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
You can use the markdown link syntax here if you like.
["smearing" the leap second](https://developers.google.com/time/smear)
I think this doesn't work in the callout syntax?
_includes/v19.2/faq/clock-synchronization-effects.md, line 11 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as before.
Done.
_includes/v19.2/faq/clock-synchronization-effects.md, line 15 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as before.
Done.
_includes/v19.2/prod-deployment/insecure-scale-cluster.md, line 37 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
--insecureseems to be missing now.
Done.
There was a problem hiding this comment.
Reviewed 9 of 20 files at r1, 10 of 10 files at r3.
Reviewable status:
_includes/v19.1/faq/clock-synchronization-effects.md, line 7 at r3 (raw file):
### Considerations When setting up clock synchronization:
I'd rewrite this whole section:
When setting up clock synchronization:
-
All nodes in the cluster must be synced to the same time sources, or to different sources that implement leap second smearing in the same way. For example, Google and Amazon have time sources that are compatible with each other (both implement leap second smearing in the same way), but they are incompatible with the default NTP pool (which does not implement leap second smearing)
-
For nodes running in AWS, we recommend Amazon Time Sync Service. For nodes running in GCP, we recommend Google's internal NTP service. For nodes running elsewhere, we recommend Google Public NTP. Note that the Google and Amazon time services can be mixed with each other, but they cannot be mixed with other time services (unless you have verified leap second behavior), so either all of your nodes should use one of these services or none of them should.
-
If you do not want to use the Google or Amazon time sources, you can use
chronyand enable client-side leap smearing... -
Do not run more than one clock sync service...
_includes/v19.1/prod-deployment/insecure-scale-cluster.md, line 32 at r3 (raw file):
If you get a permissions error, prefix the command with `sudo`. 4. Run the [`cockroach start`](start-a-node.html) command, pointing `--advertise-addr` to the new node and `--join` to the three existing nodes (also include `--locality` if you set it earlier).
Nit: the --join flag "points to" nodes, but I wouldn't use that verb for --advertise-addr. I'd say "passing the new node's address as the --advertise-addr flag and pointing --join to the three existing nodes.
_includes/v19.1/prod-deployment/insecure-test-cluster.md, line 3 at r3 (raw file):
CockroachDB replicates and distributes data behind-the-scenes and uses a [Gossip protocol](https://en.wikipedia.org/wiki/Gossip_protocol) to enable each node to locate data across the cluster. Once a cluster is live, any node can be used as a SQL gateway. When using a load balancer, you must issue commands directly to the load balancer, which then routes traffic to the nodes.
I'd say "may" (or maybe "should"?) instead of "must". Not all deployments with a load balancer will forbid direct access to the nodes.
_includes/v19.1/prod-deployment/secure-generate-certificates.md, line 56 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Good sleuthing here.
Personally I'd do an ssh-add on the keyfile and then use the same commands as the non-AWS case.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 81 at r3 (raw file):
- [Configure each GCE instance to use Google's internal NTP service](https://cloud.google.com/compute/docs/instances/managing-instances#configure_ntp_for_your_instances). - If you plan to run a hybrid cluster across GCE and other cloud providers or environments, all AWS machines should use the internal [Amazon Time Sync Service](http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/set-time.html#configure-amazon-time-service), and all other non-GCE machines should use [Google's external NTP service](deploy-cockroachdb-on-digital-ocean.html#step-2-synchronize-clocks).
Rewrite this similar to what I had above - the instructions are now the same whether you plan to run a hybrid cluster or not.
v19.1/deploy-cockroachdb-on-aws.md, line 98 at r3 (raw file):
Protocol | TCP Port Range | **8080** Source | The CIDR of your VPC (e.g., 10.12.0.0/16)
Nit: CIDR isn't a noun: "The IP range of your VPC in CIDR notation".
v19.1/deploy-cockroachdb-on-aws.md, line 117 at r3 (raw file):
1. [Add AWS load balancing](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-increase-availability.html#scale-and-load-balance). Be sure to: - Select a **Network Load Balancer** (not an Application Load Balancer, as in the above instructions) and use the ports we specify below.
Yes, I believe an NLB is required instead of an ALB.
There was a problem hiding this comment.
Reviewable status:
_includes/v19.1/faq/clock-synchronization-effects.md, line 7 at r3 (raw file):
Previously, bdarnell (Ben Darnell) wrote…
I'd rewrite this whole section:
When setting up clock synchronization:
All nodes in the cluster must be synced to the same time sources, or to different sources that implement leap second smearing in the same way. For example, Google and Amazon have time sources that are compatible with each other (both implement leap second smearing in the same way), but they are incompatible with the default NTP pool (which does not implement leap second smearing)
For nodes running in AWS, we recommend Amazon Time Sync Service. For nodes running in GCP, we recommend Google's internal NTP service. For nodes running elsewhere, we recommend Google Public NTP. Note that the Google and Amazon time services can be mixed with each other, but they cannot be mixed with other time services (unless you have verified leap second behavior), so either all of your nodes should use one of these services or none of them should.
If you do not want to use the Google or Amazon time sources, you can use
chronyand enable client-side leap smearing...Do not run more than one clock sync service...
Thanks! For the last line in the second bullet point, is the following tweak accurate?
"Either all of your nodes should use the Google and Amazon services, or none of them should."
_includes/v19.1/prod-deployment/insecure-test-cluster.md, line 19 at r2 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Looks good. Thanks.
Done.
_includes/v19.1/prod-deployment/insecure-test-cluster.md, line 3 at r3 (raw file):
Previously, bdarnell (Ben Darnell) wrote…
I'd say "may" (or maybe "should"?) instead of "must". Not all deployments with a load balancer will forbid direct access to the nodes.
Done.
_includes/v19.1/prod-deployment/secure-generate-certificates.md, line 56 at r2 (raw file):
Previously, bdarnell (Ben Darnell) wrote…
Personally I'd do an
ssh-addon the keyfile and then use the same commands as the non-AWS case.
Oh, I wasn't familiar with ssh-add. Should it look like this?
ssh-add /path/<key file>.pem
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 81 at r3 (raw file):
Previously, bdarnell (Ben Darnell) wrote…
Rewrite this similar to what I had above - the instructions are now the same whether you plan to run a hybrid cluster or not.
I thought it would be cleanest to link directly to the Production Checklist instructions. I added some verbiage for this and also added the link to the Digital Ocean and Azure guides.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 97 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as above.
Done.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 97 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as before.
Done.
v19.1/deploy-cockroachdb-on-aws.md, line 117 at r3 (raw file):
Previously, bdarnell (Ben Darnell) wrote…
Yes, I believe an NLB is required instead of an ALB.
Done.
There was a problem hiding this comment.
Reviewable status:
_includes/v19.1/faq/clock-synchronization-effects.md, line 7 at r3 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Thanks! For the last line in the second bullet point, is the following tweak accurate?
"Either all of your nodes should use the Google and Amazon services, or none of them should."
Yes, that's right.
_includes/v19.1/prod-deployment/secure-generate-certificates.md, line 56 at r2 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Oh, I wasn't familiar with
ssh-add. Should it look like this?
ssh-add /path/<key file>.pem
I think so, but you should probably test it. (what I actually do is add stuff to .ssh/config so it happens automatically)
There was a problem hiding this comment.
Reviewable status:
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 81 at r3 (raw file):
Previously, taroface (Ryan Kuo) wrote…
I thought it would be cleanest to link directly to the Production Checklist instructions. I added some verbiage for this and also added the link to the Digital Ocean and Azure guides.
Hmm, I think the context is now missing here. How about something like this?
- If you plan to run a hybrid cluster across GCE and other cloud providers or environments, note that all of the nodes must be synced to the same time source, or to different sources that implement leap second smearing in the same way. See the [Production Checklist](recommended-production-settings.html#considerations) for details.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 84 at r3 (raw file):
Previously, taroface (Ryan Kuo) wrote…
I think this doesn't work in the callout syntax?
You can use markdown as long as the opening and closing liquid tags are on their own lines.
Mentioned at the very end of this section of the style guide: https://github.com/cockroachdb/docs/blob/master/STYLE.md#tips-notes-and-warnings
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 97 at r3 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Done.
Same as above. Feel like context is now missing for last bullet.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 63 at r5 (raw file):
~~~ {{site.data.alerts.callout_info}}We recommend Google's NTP service because it handles <a href="https://developers.google.com/time/smear">"smearing" the leap second</a>. If you use a different NTP service that doesn't smear the leap second, be sure to configure client-side smearing in the same way on each machine. See the <a href="recommended-production-settings.html#considerations">Production Checklist</a> for details.{{site.data.alerts.end}}
As mentioned in another PR, let's put opening and closing tags for callouts on their own lines so contents in the middle can be markdown-formatted instead of html. This predates you, but when you're updating a callout, might as well update the formatting.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 162 at r5 (raw file):
~~~ {{site.data.alerts.callout_info}}We recommend Google's NTP service because it handles <a href="https://developers.google.com/time/smear">"smearing" the leap second</a>. If you use a different NTP service that doesn't smear the leap second, be sure to configure client-side smearing in the same way on each machine. See the <a href="recommended-production-settings.html#considerations">Production Checklist</a> for details.{{site.data.alerts.end}}
Same as above.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 97 at r3 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Done.
Same as above for last bullet.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 63 at r5 (raw file):
~~~ {{site.data.alerts.callout_info}}We recommend Google's NTP service because it handles <a href="https://developers.google.com/time/smear">"smearing" the leap second</a>. If you use a different NTP service that doesn't smear the leap second, be sure to configure client-side smearing in the same way on each machine. See the <a href="recommended-production-settings.html#considerations">Production Checklist</a> for details.{{site.data.alerts.end}}
Same as above.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 162 at r5 (raw file):
~~~ {{site.data.alerts.callout_info}}We recommend Google's NTP service because it handles <a href="https://developers.google.com/time/smear">"smearing" the leap second</a>. If you use a different NTP service that doesn't smear the leap second, be sure to configure client-side smearing in the same way on each machine. See the <a href="recommended-production-settings.html#considerations">Production Checklist</a> for details.{{site.data.alerts.end}}
Same as above.
v19.1/deploy-cockroachdb-on-aws.md, line 117 at r3 (raw file):
Previously, taroface (Ryan Kuo) wrote…
Done.
I think "as in the above instructions" is confusing. At least it confused me; I looked elsewhere on the page for something. Instead, maybe go with "not an Application Load Balancer, as in the linked instructions".
Is there not an NLB-specific link we can use instead?
v19.1/deploy-cockroachdb-on-aws-insecure.md, line 114 at r5 (raw file):
1. [Add AWS load balancing](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-increase-availability.html#scale-and-load-balance). Be sure to: - Select a **Network Load Balancer** (not an Application Load Balancer, as in the above instructions) and use the ports we specify below.
Same as above.
v19.2/deploy-cockroachdb-on-aws.md, line 117 at r5 (raw file):
1. [Add AWS load balancing](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-increase-availability.html#scale-and-load-balance). Be sure to: - Select a **Network Load Balancer** (not an Application Load Balancer, as in the above instructions) and use the ports we specify below.
Same as above.
v19.2/deploy-cockroachdb-on-aws-insecure.md, line 114 at r5 (raw file):
1. [Add AWS load balancing](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-increase-availability.html#scale-and-load-balance). Be sure to: - Select a **Network Load Balancer** (not an Application Load Balancer, as in the above instructions) and use the ports we specify below.
Same as above.
There was a problem hiding this comment.
Reviewable status:
_includes/v19.1/faq/clock-synchronization-effects.md, line 7 at r3 (raw file):
Previously, bdarnell (Ben Darnell) wrote…
Yes, that's right.
Done.
_includes/v19.1/prod-deployment/secure-generate-certificates.md, line 56 at r2 (raw file):
Previously, bdarnell (Ben Darnell) wrote…
I think so, but you should probably test it. (what I actually do is add stuff to
.ssh/configso it happens automatically)
Done.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 81 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Hmm, I think the context is now missing here. How about something like this?
- If you plan to run a hybrid cluster across GCE and other cloud providers or environments, note that all of the nodes must be synced to the same time source, or to different sources that implement leap second smearing in the same way. See the [Production Checklist](recommended-production-settings.html#considerations) for details.
Understood, and sounds good. I had taken Ben's input to mean that we don't need to specifically mention "hybrid clusters" anymore, but I think that was too big an assumption.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 84 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
You can use markdown as long as the opening and closing liquid tags are on their own lines.
Mentioned at the very end of this section of the style guide: https://github.com/cockroachdb/docs/blob/master/STYLE.md#tips-notes-and-warnings
Done.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 63 at r5 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
As mentioned in another PR, let's put opening and closing tags for callouts on their own lines so contents in the middle can be markdown-formatted instead of html. This predates you, but when you're updating a callout, might as well update the formatting.
Done.
_includes/v19.1/prod-deployment/synchronize-clocks.md, line 162 at r5 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as above.
Done.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 63 at r5 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as above.
Done.
_includes/v19.2/prod-deployment/synchronize-clocks.md, line 162 at r5 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as above.
Done.
v19.1/deploy-cockroachdb-on-aws.md, line 117 at r3 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
I think "as in the above instructions" is confusing. At least it confused me; I looked elsewhere on the page for something. Instead, maybe go with "not an Application Load Balancer, as in the linked instructions".
Is there not an NLB-specific link we can use instead?
Of course. Found an NLB-specific link and took out the mention of ALB!
v19.1/deploy-cockroachdb-on-aws-insecure.md, line 114 at r5 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as above.
Done.
v19.2/deploy-cockroachdb-on-aws.md, line 117 at r5 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as above.
Done.
v19.2/deploy-cockroachdb-on-aws-insecure.md, line 114 at r5 (raw file):
Previously, jseldess (Jesse Seldess) wrote…
Same as above.
Done.
fixes #5661
fixes #4847
fixes #4659
fixes #5317