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

Add TA-131639 #18981

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add TA-131639 #18981

wants to merge 3 commits into from

Conversation

rmloveland
Copy link
Contributor

Fixes DOC-11251

Copy link

github-actions bot commented Oct 3, 2024

Files changed:

Copy link

netlify bot commented Oct 3, 2024

Deploy Preview for cockroachdb-interactivetutorials-docs canceled.

Name Link
🔨 Latest commit f897fa6
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-interactivetutorials-docs/deploys/66feef1d9106750008e70c62

Copy link

netlify bot commented Oct 3, 2024

Deploy Preview for cockroachdb-api-docs canceled.

Name Link
🔨 Latest commit f897fa6
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-api-docs/deploys/66feef1df34a340008a5f7e0

Copy link

netlify bot commented Oct 3, 2024

Deploy Preview for cockroachdb-docs failed. Why did it fail? →

Name Link
🔨 Latest commit f897fa6
🔍 Latest deploy log https://app.netlify.com/sites/cockroachdb-docs/deploys/66feef1d51a9eb0008100f80

@rmloveland
Copy link
Contributor Author

TFTR @arulajmani !

(FYI for anyone reading, the reason this build is failing is b/c the link to v23.1.27 release notes does not exist yet, once #18978 merges that link will exist and I'll trigger re-build of this)

@nvanbenschoten
Copy link
Member

@rmloveland please hold off on merging this for now. We need to find a better way to eliminate false positives without manual inspection of the logs before publishing. If we don’t do a good enough job, we’ll create a lot of undue concern.

@rmloveland rmloveland removed the request for review from florence-crl October 3, 2024 17:04
@rmloveland
Copy link
Contributor Author

rmloveland commented Oct 3, 2024

please hold off on merging this for now. We need to find a better way to eliminate false positives without manual inspection of the logs before publishing. If we don’t do a good enough job, we’ll create a lot of undue concern.

👍

src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
applied lease after .* replication lag, client traffic may have been delayed.*epo=.*prev.*exp=.*Request.*
~~~

If all three of these log lines are present and the timestamp of the third log is less than the specified "replication lag" after the timestamp of the second log, then it is possible that a lost write occurred on the range identified in the third log line.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nvanbenschoten - wdyt about putting a few of those examples that you had from the original issue investigation to really drive home what we're looking for. Maybe we can "bold" the bits of interesting (does markdown support bold within a code block??).

Copy link
Contributor Author

@rmloveland rmloveland Oct 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can "bold" the bits of interesting (does markdown support bold within a code block??).

AFAICT the answer is no but we can do ASCII gymnastics like

INFO: the value of foo=bar at time t=123
                   ^^^^^^^
                LOOK FOR THIS

Copy link
Contributor

@florence-crl florence-crl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider my suggestions. thanks!

src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
src/current/advisories/a131639.md Outdated Show resolved Hide resolved
@rmloveland
Copy link
Contributor Author

Please consider my suggestions. thanks!

thanks @florence-crl! applied all of your suggestions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants