-
Notifications
You must be signed in to change notification settings - Fork 473
docs: in ALTER CLUSTER docs, remove limitations around sources/sinks #32960
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
Conversation
We have the code for this ready but it's not yet rolled out via LD flag. This change should be merged once we GA the flag change. Closes MaterializeInc/database-issues#9016
* For clusters that **do not contain sources or sinks**, it's possible to avoid | ||
downtime by performing a [zero-downtime cluster | ||
resizing](#zero-downtime-cluster-resizing). | ||
TODO: Remove this section in favour of what we have just below? |
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.
We should also change:
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.
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.
Also, just wondering ... in the https://github.com/MaterializeInc/materialize/blob/main/doc/user/content/concepts/clusters.md#fault-tolerance ... should we mention that for source/sink clusters ... replication factor does not improve fault tolerance?
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.
Sorry -- a little scattered attention this morning -- but these should also change to use the option:
- https://github.com/MaterializeInc/materialize/blob/main/doc/user/layouts/shortcodes/postgres-direct/right-size-the-cluster.html
- https://github.com/MaterializeInc/materialize/blob/main/doc/user/layouts/shortcodes/mysql-direct/right-size-the-cluster.html
- https://github.com/MaterializeInc/materialize/blob/main/doc/user/layouts/shortcodes/kafka/cockroachdb/create-a-cluster.html
- https://github.com/MaterializeInc/materialize/blob/main/doc/user/layouts/shortcodes/sql-server-direct/right-size-the-cluster.html
- https://github.com/MaterializeInc/materialize/blob/main/doc/user/shared-content/resize-cluster-for-snapshotting.md
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.
Thank you for catching these! 🙇♂️
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.
np ❤️ ... One last thing ... this seems like such a wow/celebratory change, so was wondering if we should also highlight it in the main landing page in the what's new section ...
https://github.com/MaterializeInc/materialize/blob/main/doc/user/content/_index.md#whats-new
* For clusters that **do not contain sources or sinks**, it's possible to avoid | ||
downtime by performing a [zero-downtime cluster | ||
resizing](#zero-downtime-cluster-resizing). | ||
TODO: Remove this section in favour of what we have just below? |
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.
``` | ||
```mzsql | ||
ALTER CLUSTER c1 SET (SIZE '100cc'); | ||
``` |
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.
Here we might need to state that you can have downtime if your cluster has sources or sinks?
Depending on the type of objects in a cluster, a resizing operation might incur | ||
**downtime**. | ||
It's possible to avoid downtime by performing a [zero-downtime cluster | ||
resizing](#zero-downtime-cluster-resizing). |
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.
So ... we probably need to mention downtime as a lead-in.
Maybe: "Resizing operation can incur downtime unless used with WAIT UNTIL READY
option. See zero-downtime cluster resizing for details." or something like that.
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.
Left one comment about the limitation.
which incurs **no downtime**. Instead of restarting the cluster, this approach | ||
spins up an additional cluster replica under the covers with the desired new | ||
size, waits for the replica to be hydrated, and then replaces the original | ||
replica. |
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.
Should we mention the limitation - that you must keep your SQL/console session open?
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 already has this:
fail. Additionally, any connection interruption or statement cancelation will |
If you like, I can create a separate PR that makes it more explicit that you need to keep your session open. I would do it separately because I can only merge this here PR once I enabled multi-replica sources in production.
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.
Yes, please. If we could add a file with the blurb into the https://github.com/MaterializeInc/materialize/tree/main/doc/user/shared-content/ directory .. then we can add it both in the details section and the example using the {{< include-md file="shared-content/some-file.md" >}}
(needs the .md extension)
We have the code for this ready but it's not yet rolled out via LD flag. This change should be merged once we GA the flag change.
Closes https://github.com/MaterializeInc/database-issues/issues/9016