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

Import data from long-term database #208

Merged
merged 8 commits into from
Jan 20, 2018
Merged

Import data from long-term database #208

merged 8 commits into from
Jan 20, 2018

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Jan 20, 2018

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • 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


This pull requests implements some general improvements for FTL.

Changes:

  • Instead of completely parsing the log files pihole.log and pihole.log.1 each time FTL starts up, we read the already analyzed and categorized data from the long term database. We only read those queries from pihole.log that are not already present in the database. This enhances the startup speed of FTL as well as provides some functionality we will be using with FTL in the future.
  • Instead of counting all forward activity, we only count the first server a request is forwarded to. This will change the appearance off the Forward Destinations (integrated) plot and is implemented to acknowledge the fact that we store only one forward destination per query in the database.

Minor fixes:

  • In case we cached a <CNAME> record but still have to ask the upstream servers for the actual IP addresses, we don't count this query as being cached but as being forwarded. Note that this will reduce the local forward destination count (it has been too high so far).
  • Remove unnecessarily duplicated validate_access() calls at some places for a slight performance increase.

This template was created based on the work of udemy-dl.

…ed it in database.c)

Signed-off-by: DL6ER <dl6er@dl6er.de>
…TL. Fall back to log interpretation if database is not available

Signed-off-by: DL6ER <dl6er@dl6er.de>
Signed-off-by: DL6ER <dl6er@dl6er.de>
…rvers for the actual IPs. In think case we have to decrease the "cached" counter

Signed-off-by: DL6ER <dl6er@dl6er.de>
…s counted

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER added this to the v3.0 milestone Jan 20, 2018
Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER changed the title [WIP] Import data from long-term database Import data from long-term database Jan 20, 2018
@DL6ER DL6ER requested a review from a team January 20, 2018 14:23
queries[queryID].timeidx = timeidx;
queries[queryID].valid = true; // Mark this as a valid query (false = it has been garbage collected and should be skipped)
queries[queryID].db = true; // Mark this as already present in the database
queries[queryID].id = 0; // This is dnsmasq's internal ID. We don't store it in the database
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make sure, the dnsmasq query IDs don't start at 0, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it starts with 1.

@@ -587,27 +578,21 @@ void process_pihole_log(int file)
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing free(forward)

parser.c Outdated
@@ -587,27 +578,21 @@ void process_pihole_log(int file)
continue;
}

// Count only if current query has not been counted so far
if(queries[i].complete)
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing free(forward)

extracttimestamp(readbuffer, &querytimestamp, &overTimetimestamp);

// Check if this query has already been imported from the database
if(querytimestamp < lastDBimportedtimestamp) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

There are more of this line below, but since this is run on every line, the others would just be redundant?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yes, they shouldn't be there, seems like I forgot to remove them at some places.

Signed-off-by: DL6ER <dl6er@dl6er.de>
@DL6ER DL6ER merged commit 9fda0e6 into development Jan 20, 2018
@DL6ER DL6ER deleted the new/loadFromDB branch January 20, 2018 19:59
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.

2 participants