Skip to content
This repository has been archived by the owner on Oct 18, 2023. It is now read-only.

hrana: add diagnostics for connections #729

Merged
merged 6 commits into from
Oct 5, 2023
Merged

Conversation

psarna
Copy link
Contributor

@psarna psarna commented Oct 4, 2023

This commit adds a /v1/diagnostics admin endpoint which prints various information about current hrana-over-http connections.

Draft, because the diagnostics are currently in a very debuggy format, and I'm figuring out if we can make it more human-readable. Still, they're enough to determine if something is holding a lock via an abandoned hrana-over-http stream.

Example:

$ curl -s http://localhost:8082/v1/diagnostics | jq
[
  "expired",
  "(conn: Mutex { data: <locked> }, timeout_ms: 0, stolen: true)",
  "expired",
  "(conn: Mutex { data: <locked> }, timeout_ms: 4291, stolen: false)",
  "expired",
  "expired",
  "<no-transaction>",
  "acquired"
]

@psarna psarna requested review from penberg and MarinPostma October 4, 2023 09:00
@psarna psarna marked this pull request as ready for review October 4, 2023 10:31
Copy link
Collaborator

@MarinPostma MarinPostma left a comment

Choose a reason for hiding this comment

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

One small thing, otherwise LGTM.

We should go over that in the future and design a way to have diagnostic probes that don't couple unrelated modules too much

sqld/src/http/admin/mod.rs Outdated Show resolved Hide resolved
@MarinPostma
Copy link
Collaborator

@psarna, can you rebase maybe?

psarna and others added 5 commits October 5, 2023 09:19
This commit adds a /v2/diagnostics endpoint which prints
various information about current hrana-over-http connections.

Draft, because the diagnostics are currently in a very debuggy
format, and I'm figuring out if we can make it more human-readable.
Still, they're enough to determine if something is holding a lock
via an abandoned hrana-over-http stream.

Example:
```
$ curl -s http://localhost:8080/v2/diagnostics | jq
[
  "expired",
  "expired",
  "expired",
  "expired",
  "expired",
  "(conn: Mutex { data: <locked> }, timeout_ms: 872, stolen: false)",
  "(conn: Mutex { data: <locked> }, timeout_ms: 0, stolen: true)"
]
```
Co-authored-by: ad hoc <postma.marin@protonmail.com>
@psarna
Copy link
Contributor Author

psarna commented Oct 5, 2023

This PR should have absolutely 0 to do with with bottomless, and yet I see bottomless test failures, that always pass for me locally. Maybe we have some kind of regression, let me investigate

@psarna
Copy link
Contributor Author

psarna commented Oct 5, 2023

Oh wait, now they failed for me locally too, good

@@ -199,6 +201,9 @@ unsafe impl WalHook for ReplicationLoggerHook {
"Ignoring a checkpoint request weaker than TRUNCATE: {}",
emode
);
if emode == SQLITE_CHECKPOINT_PASSIVE {
return SQLITE_OK;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh wow, that's leftovers from my early tests, it should not be there at all

@psarna psarna merged commit 7b5b4f2 into libsql:main Oct 5, 2023
7 of 8 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants