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

Include Lock:Timeout (timeout > 0) Event Class in sqlserver input #6978

Closed
sawo1337 opened this issue Feb 4, 2020 · 7 comments · Fixed by #7808
Closed

Include Lock:Timeout (timeout > 0) Event Class in sqlserver input #6978

sawo1337 opened this issue Feb 4, 2020 · 7 comments · Fixed by #7808
Labels
area/sqlserver feature request Requests for new plugin and for new features to existing plugins

Comments

@sawo1337
Copy link

sawo1337 commented Feb 4, 2020

Feature Request

The current metric only shows Lock Timeouts/sec, which includes internal lock probes that may not be a problem at all. In a live environment, you may get 100s of lock timeouts/sec, but actually no lock timeouts that are longer than 0.

Proposal:

Add Lock Timeouts (timeout > 0)/sec to the sqlserver_performance metric

Current behavior:

Only Lock Timeouts/sec is being gathered

Desired behavior:

Add Lock Timeouts (timeout > 0)/sec to the sqlserver_performance metric

Use case:

If you want to only see locks that are actually causing issues, it is better to skip locks where the timeout is 0. From MSDN:
image
MSDN Article

@danielnelson danielnelson added area/sqlserver feature request Requests for new plugin and for new features to existing plugins labels Feb 4, 2020
@danielnelson
Copy link
Contributor

Seems like a good idea to me, what do you think @denzilribeiro?

@denzilribeiro
Copy link
Contributor

What we have is just a perf counter in which Lock timeout/sec is a rate counter you cannot just ignore /exclude values. Look at the cntr_type column in sys.dm_os_performance_counters
To use waitstats analysis or waits on LCK to troubleshoot any lock related issue/s. I am not totally sure if you are proposing adding XEvent class? that is more tracing not monitoring.

@sawo1337
Copy link
Author

sawo1337 commented Feb 4, 2020

There is a separate counter that is actually named "Lock Timeouts (timeout > 0)/sec" - it is available by default, it is just a matter of including it as it gives much more realistic values.

@denzilribeiro
Copy link
Contributor

That is totally fine to replace this one with that .

@sawo1337
Copy link
Author

sawo1337 commented Feb 4, 2020

Exactly, here is an output from production server with pretty light load:
image

Lock timeouts is giving pretty crazy values due to internal locks, but the actual locks with wait over 0 is giving some reasonable output. This is why Microsoft suggests watching Lock Timeouts (timeout > 0)/sec instead.

@sawo1337
Copy link
Author

sawo1337 commented Feb 6, 2020

Proposed change in sqlserver.go:
line 639:
'Lock Timeouts/sec',
becomes:
'Lock Timeouts (timeout > 0)/sec,
Or add this as an additional counter on line 640 and keep the existing counter at line 639

@danielnelson
Copy link
Contributor

@sawo1337 Can you open a pull request with both counters?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sqlserver feature request Requests for new plugin and for new features to existing plugins
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants