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

feat: add request id as comment to all queries #44884

Merged
merged 1 commit into from
Apr 22, 2024
Merged

Conversation

icewind1991
Copy link
Member

Having the request id in the sql can be useful for various database debugging

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Apr 17, 2024
@icewind1991 icewind1991 added this to the Nextcloud 30 milestone Apr 17, 2024
@icewind1991 icewind1991 requested review from nickvergessen, juliushaertl, a team, ArtificialOwl, Fenn-CS and yemkareems and removed request for a team April 17, 2024 12:29
@icewind1991
Copy link
Member Author

Silly me, expecting various SQL implementations to support the same comment format...

@icewind1991 icewind1991 added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Apr 17, 2024
@juliushaertl
Copy link
Member

After todays call with the db expert we could even consider this for selects (if the query cache is disabled), which would be recommended anyways these days for MySQL/MariaDB.

@icewind1991
Copy link
Member Author

which would be recommended anyways these days for MySQL/MariaDB.

So there would be no downside for always having this on?

@juliushaertl
Copy link
Member

I think we should not have the request id there by default but could consider it as a config option if an admin is sure that they have the query cache enabled, otherwise there might be quite an overhead invalidating it with every query.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
Copy link
Member Author

Put the request id behind the db.log_request_id system flag

@icewind1991 icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 17, 2024
@icewind1991
Copy link
Member Author

/backport to stable29

@icewind1991
Copy link
Member Author

/backport to stable28

@icewind1991
Copy link
Member Author

/backport to stable27

@icewind1991 icewind1991 merged commit deac58a into master Apr 22, 2024
157 checks passed
@icewind1991 icewind1991 deleted the query-req-id branch April 22, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants