Skip to content
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

crosscluster/logical: permanent job errors should fail LDR job #139487

Merged
merged 2 commits into from
Jan 22, 2025

Conversation

msbutler
Copy link
Collaborator

Previously, permanent job errors would pause the LDR job, like PCR. Since LDR
doesn't have a cutover step, we should instead fail the job to provide a
clearer UX to the user.

Epic: none

Release note: none

@msbutler msbutler self-assigned this Jan 21, 2025
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@msbutler msbutler force-pushed the butler-heartbeat-err branch from 0dcbaa5 to d84a3be Compare January 21, 2025 17:45
@msbutler msbutler marked this pull request as ready for review January 21, 2025 18:19
@msbutler msbutler requested a review from a team as a code owner January 21, 2025 18:19
@msbutler msbutler requested review from jeffswenson and dt and removed request for a team January 21, 2025 18:19
@msbutler msbutler added the backport-25.1.x Flags PRs that need to be backported to 25.1 label Jan 22, 2025
@msbutler
Copy link
Collaborator Author

TFTR!

bors r=kev-cao

craig bot pushed a commit that referenced this pull request Jan 22, 2025
139342: ui: fix statement diag reports when min exec latency is null r=kyle-a-wong a=kyle-a-wong

A bug in db console was resulting in statement diagnostics reports to not work as intended. As a result, activating diagnostics didn't result in the intended state change which showed a user that a diagnostics report is running or downloadble.

This was happening in edge cases where reports "minExecutionLatency" response field was null, but the db console expected it to be populated. Now, db console should handle this edge case.

Fixes: #139340
Epic: none
Release note (bug fix): Fixes a bug where sometimes activating diagnostics for sql activity appears unresponsive, with no state or status update upon activating. Now, the status should always reflect that diagnosticsa are active or that a statement bundle is downloadable.

139487: crosscluster/logical: permanent job errors should fail LDR job r=kev-cao a=msbutler

Previously, permanent job errors would pause the LDR job, like PCR. Since LDR
doesn't have a cutover step, we should instead fail the job to provide a
clearer UX to the user.

Epic: none

Release note: none

139491: crosscluster/physical: wait for sip shutdown before cutover r=kev-cao a=msbutler

Informs #136588

Release note: none

Co-authored-by: Kyle Wong <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
@craig
Copy link
Contributor

craig bot commented Jan 22, 2025

Build failed (retrying...):

craig bot pushed a commit that referenced this pull request Jan 22, 2025
139487: crosscluster/logical: permanent job errors should fail LDR job r=kev-cao a=msbutler

Previously, permanent job errors would pause the LDR job, like PCR. Since LDR
doesn't have a cutover step, we should instead fail the job to provide a
clearer UX to the user.

Epic: none

Release note: none

139559: rttanalysis: loosen jobs rtts r=dt a=dt

Looking at the traces of recent failures of this test, I see 15 batch requests that all look correct to me even though the test says 12. I don't know why it sometimes passes as-is but given that as many as 15 look correct, it should allow for that.

Release note: none.
Epic: none.

Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: David Taylor <[email protected]>
@craig
Copy link
Contributor

craig bot commented Jan 22, 2025

Build failed (retrying...):

Previously, permanent job errors would pause the LDR job, like PCR. Since LDR
doesn't have a cutover step, we should instead fail the job to provide a
clearer UX to the user.

Epic: none

Release note: none
@msbutler msbutler force-pushed the butler-heartbeat-err branch from d84a3be to 4139bf6 Compare January 22, 2025 19:06
@craig
Copy link
Contributor

craig bot commented Jan 22, 2025

Canceled.

@msbutler
Copy link
Collaborator Author

bors r+

@craig craig bot merged commit 58cf933 into cockroachdb:master Jan 22, 2025
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-25.1.x Flags PRs that need to be backported to 25.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants