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

Prevent excessive reply times in the Query Log after restarting #1297

Merged
merged 1 commit into from
Feb 5, 2022

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Feb 5, 2022

By submitting this pull request, I confirm the following:

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10


The query->reply_time variable serves two purposes: (1) When sending a query upstream, we store the current time in this field. (2) When the reply arrives, we compute the difference and store it in the same field. We memorize the transition from (1) to (2) by setting the flag query->flags.response_calculated to true.

Currently (broken in development, not in master), we store the reply_time field unconditionally, hence, for a few queries which are still being processed upstream at restart, we store the real time instead of the response. FTL incorrectly interprets the (homogeneous!) timestamp as difference and telly you the query needed quadrillions of milliseconds to get answered.

We fix this small bug by instead storing NULL in the REPLY_TIME column when the reply time is not available. FTL already handles this case well when loading from the database: the default of query->flags.response_calculated is false. A NULL value in this field does not change this default.

@DL6ER
Copy link
Member Author

DL6ER commented Feb 5, 2022

No documentation change is required. The current documentation already says that the field is allowed to by "empty" which means NULL for SQL with column type REAL.

@yubiuser yubiuser merged commit 76d3328 into development Feb 5, 2022
@yubiuser yubiuser deleted the fix/DB_upstream_reply branch February 5, 2022 11:12
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-14-web-v5-11-and-core-v5-9-released/53529/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants