Track max transaction elapsed time across multiple transactions #2504
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8318582639
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| SDEBUG("Peeking at '" << request.methodLine << "' with priority: " << command->priority); | ||
| command->peekCount++; | ||
| _db.resetMaxTransactionElapsed(); | ||
| _db.setTimeout(_getRemainingTime(command, false)); |
There was a problem hiding this comment.
This reset is only performed when peekCommand runs. However, BedrockServer can skip peek for commands with existing HTTPS requests (see if (command->repeek || !command->httpsRequests.size())), and those commands go straight to processCommand. With the new max() behavior in commit/rollback, _totalTransactionElapsed can now carry over from a previous command when peek is skipped, so timing logs report an inflated max unrelated to the current command. To avoid cross-command contamination, the reset needs to run at the start of any command path (e.g., before processing when peek is skipped), not just in peek.
Useful? React with 👍 / 👎.
When a command runs multiple transactions (e.g., peek then process), _totalTransactionElapsed was only storing the last transaction's time. This meant slow transactions in earlier phases were silently lost in timing logs. Use max() in both rollback and commit so the highest transaction time is preserved. Reset the counter at command boundaries (start of peekCommand) so it doesn't carry over between commands on the same shared db handle. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
danieldoglas
force-pushed
the
dsilva-track-max-transaction-elapsed-v2
branch
from
8318582 to
4e07f33
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Transaction Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
NVM this, I forgot that we log the line every time we call rollback, so we are keepting track of all transactions, not only the last one. |
1 participant
Summary
_totalTransactionElapsedwas only storing the last transaction's time when a command ran multiple transactions (e.g., when peek makes httpsRequests and the transaction is closed and reopened). This meant the slowest transaction could be silently lost in timing logs.max()so the highest transaction time is preserved across all transactions within a single command.resetMaxTransactionElapsed()and call it at db assignment boundaries (pool handle initialization and sync thread command loop) so stale timing doesn't carry over between commands on the same db handle.🤖 Generated with Claude Code