c2c: prevent NPE in SHOW TENANT WITH REPLICATION STATUS #120434
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the user ran SHOW TENANT ... WITH REPLICATION STATUS before the stream ingestion job pts was set, a null pointer exception would occur and crash the node. This patch fixes this bug. Epic: None Release note: none
msbutler
added
T-disaster-recovery
backport-23.1.x
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
stevendanna
approved these changes
Mar 15, 2024
|
It should show the zero timestamp. This field isn't a pointer to it is safe to leave unset. cockroach/pkg/sql/show_tenant.go Line 36 in 78b3141 |
|
TFTR! bors r=stevendanna |
|
Build succeeded: |
This was referenced Mar 15, 2024
msbutler
added a commit
to msbutler/cockroach
that referenced
this pull request
Mar 19, 2024
This patch adds a test for the fix in cockroachdb#120434 Epic: none Release note: none
craig bot
pushed a commit
that referenced
this pull request
Mar 20, 2024
120428: pkg/util/log: enable bufferedSink usage with fileSink as experimental r=dhartunian,jbowens a=abarganier Informs: #72452 Epic: CRDB-35401 For some time, two buffering mechanisms have existed within the log package. One is controlled by `buffered-writes`, which is only used by the file sink. `buffered-writes` buffers writes until a buffer is full, after which it flushes the buffer to an underlying file. This is not an asynchronous operation. Separately, the `bufferedSink` was introduced to provide an async buffering mechanism. This was originally intended for network log sinks such as the `fluentSink` and `httpSink`. Originally, the `bufferedSink` was not allowed for use with the `fileSink` as we already had a buffering mechanism available. Today, we find ourselves in a situation where we aim to make CRDB even more resilient in the face of disk failures. However, some of these efforts like WAL failover are unable to achieve their goals if the logging subsystem isn't resilient to disk unavailability as well. Today, if a file sink is configured to write to an unavailable disk, any goroutine making a logging call to one of those sinks will hang as the underlying i/o operations used are unable to interact with the disk. This can have a severe negative impact on cluster performance and throughput, even when using `buffered-writes: true`. As a way to alleviate this pain, this patch experimentally enables the usage of the `bufferedSink` with the `fileSink`. The `bufferedSink` is used to "wrap" an underlying sink (such as a `fileSink`), and asynchronously outputs buffered data to said sink. It is especially good at shielding writers from any problems with the underlying sink, which makes it a great candidate to help us accomplish our resiliency goals when it comes to disk outage scenarios. Users can leverage the `bufferedSink` by using the `buffering` config option. For example: ``` file-defaults: buffered-writes: false buffering: max-staleness: 5s flush-trigger-size: 1.0MiB max-buffer-size: 50MiB ``` Note that we don't allow both `buffered-writes` and `buffering` to be used together. This patch adds a validation step to ensure this. Release note (ops change): Users have the ability to update their log configuration to enable asynchronous buffering of `file-group` log sinks via the `buffering` [configuration options](https://www.cockroachlabs.com/docs/stable/configure-logs#log-buffering-for-network-sinks). The `buffering` configuration can be applied to `file-defaults` or individual `file-groups` as needed. Note however that `buffering` and `buffered-writes` are incompatible for the time being. If you'd like to try the `buffering` option on a file log sink, it's necessary to explicitly set `buffered-writes: false`. For example: ``` file-defaults: buffered-writes: false buffering: max-staleness: 1s flush-trigger-size: 256KiB max-buffer-size: 50MiB ``` We recommend a `max-staleness: 1s` and `flush-trigger-size: 256KiB` when using this feature with file log sinks. 120725: streamingccl: add show tenant without pts test r=stevendanna a=msbutler This patch adds a test for the fix in #120434 Epic: none Release note: none Co-authored-by: Alex Barganier <[email protected]> Co-authored-by: Michael Butler <[email protected]>
msbutler
added a commit
to msbutler/cockroach
that referenced
this pull request
Mar 20, 2024
This patch adds a test for the fix in cockroachdb#120434 Epic: none Release note: none
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the user ran SHOW TENANT ... WITH REPLICATION STATUS before the stream ingestion job pts was set, a null pointer exception would occur and crash the node. This patch fixes this bug.
Epic: None
Release note: none