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

server: bad certificate error spams the logs #122622

Open
ajstorm opened this issue Apr 18, 2024 · 4 comments · May be fixed by #122931 or #124789
Open

server: bad certificate error spams the logs #122622

ajstorm opened this issue Apr 18, 2024 · 4 comments · May be fixed by #122931 or #124789
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience E-quick-win Likely to be a quick win for someone experienced. good first issue O-testcluster Issues found or occurred on a test cluster, i.e. a long-running internal cluster P-2 Issues/test failures with a fix SLA of 3 months T-server-and-security DB Server & Security

Comments

@ajstorm
Copy link
Collaborator

ajstorm commented Apr 18, 2024

We recently recreated the drt-large cluster but accidentally forgot to stop the workload cluster which was feeding it. As a result, the workload cluster contained mismatched certificates which wouldn't allow a SQL connection. This is all fine.

What's bad is that when running the old workload setup against the new cluster, the logs get spammed with this message:

E240418 12:56:19.337038 118941573 1@server/server_sql.go:1872 ⋮ [T1,Vsystem,n19,client=‹34.95.61.134:40008›] 41238547  serving SQL client conn: remote error: ‹tls: bad certificate›

The log message gets written to the logs more than 500 times per second, and as a result, fills up a 10MB log file twice per minute (thus wrapping the logs very quickly). It would be better if we only printed this log message periodically as we're receiving these connection requests.

Jira issue: CRDB-38004

@ajstorm ajstorm added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience good first issue T-server-and-security DB Server & Security E-quick-win Likely to be a quick win for someone experienced. O-testcluster Issues found or occurred on a test cluster, i.e. a long-running internal cluster P-2 Issues/test failures with a fix SLA of 3 months labels Apr 18, 2024
@blathers-crl blathers-crl bot added this to To do in DB Server & Security Apr 18, 2024
@vishgupt
Copy link

Hi @ajstorm, I want to work on this as my first issue. Can you provide more details about the expected behavior for this log message?

@ajstorm
Copy link
Collaborator Author

ajstorm commented Apr 22, 2024

Hi @vishgupt. Thanks for reaching out.

I believe we'll want to wrap this log message in a log.Every, as is done in this PR.

@AidanXu
Copy link

AidanXu commented Apr 23, 2024

Hey, @ajstorm, could I work on this as my first issue?

AidanXu added a commit to AidanXu/cockroach that referenced this issue Apr 23, 2024
amitrahman1026 added a commit to amitrahman1026/cockroach that referenced this issue May 29, 2024
Epic: none
Fixes: cockroachdb#122622

Release note: None
amitrahman1026 added a commit to amitrahman1026/cockroach that referenced this issue May 29, 2024
amitrahman1026 added a commit to amitrahman1026/cockroach that referenced this issue May 29, 2024
amitrahman1026 added a commit to amitrahman1026/cockroach that referenced this issue May 29, 2024
Previously, log message gets written to the logs more than 500 times
per second, and as a result, fills up a 10MB log file twice per minute
(thus wrapping the logs very quickly)

To address this, a logEvery wrapper was added to only log when needed.

Fixes: cockroachdb#122622

Release note: None
@amitrahman1026 amitrahman1026 linked a pull request May 29, 2024 that will close this issue
@amitrahman1026
Copy link

amitrahman1026 commented May 29, 2024

Hey, @ajstorm, apologies for the multiple backlinks. Saw this was getting a little stale because it seemed like @AidanXu missed out on linting, so I went ahead and did that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience E-quick-win Likely to be a quick win for someone experienced. good first issue O-testcluster Issues found or occurred on a test cluster, i.e. a long-running internal cluster P-2 Issues/test failures with a fix SLA of 3 months T-server-and-security DB Server & Security
Projects
4 participants