Refactored DNS log pagination to use LIMIT and OFFSET in QueryLogsMySql (#1317)#1702
Draft
jimstrang wants to merge 4 commits intoTechnitiumSoftware:masterfrom
Draft
Refactored DNS log pagination to use LIMIT and OFFSET in QueryLogsMySql (#1317)#1702jimstrang wants to merge 4 commits intoTechnitiumSoftware:masterfrom
jimstrang wants to merge 4 commits intoTechnitiumSoftware:masterfrom
Conversation
Replaced the previous ROW_NUMBER()-based pagination with a simpler and more efficient approach using SQL LIMIT and OFFSET clauses. This change removes unnecessary subqueries and row number calculations, improving code clarity and query performance for paginated DNS log retrieval.
Replace database ID with a calculated row number in DnsLogEntry objects, assigning sequential numbers based on the current page and sort order. This ensures each entry reflects its position in the paginated result set rather than its database ID.
Refactored QueryLogsAsync to better handle invalid page numbers, ensuring pageNumber is always within valid bounds. Database queries are now only executed if there are entries to fetch, preventing unnecessary queries and improving efficiency. Indented and grouped parameter setup and result reading logic for clarity and correctness.
Changed pageNumber check to require values >= 1 instead of >= 0, ensuring that page 0 cannot be used in log queries.
|
This would be a great PR to merge! I recently started hosting Technitium on a low-power Orange Pi 3B using MariaDB for query logs, but the queries often took 10+ seconds or timed out. I came to the same conclusion as you did in issue #1371 and had made similar changes to the code for my setup (https://github.com/laurentlbm/DnsServer/blob/a57741efd7b63aab51589f69c029d979f7a3b51c/Apps/QueryLogsMySqlApp/App.cs). It completely fixed my performance issues. |
Author
|
On my to-do list to make the same (or similar) changes to the SQL Server and SQLite projects as they have the same issue. It would be best to keep them all roughly in sync |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the log querying and pagination logic in the
QueryLogsAsyncmethod ofApp.csto simplify and improve how paginated results are fetched from MySQL. The changes replace the previous row number-based pagination with a more efficientLIMIT/OFFSETapproach, fix page number validation, and update how row numbers are assigned to log entries in the result set. Ref issue #1371.Pagination and Query Logic Improvements:
LIMITandOFFSETapproach for pagination, simplifying the query and improving performance. [1] [2]ROW_NUMBER()usage, directly ordering bydlidand applyingLIMIT/OFFSETfor result pagination.Page Number Validation:
pageNumberis set to 1 if a value less than 1 is provided, preventing invalid page numbers. [1] [2]Row Number Assignment: