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

Move state of reports to the filesystem #91

Merged
merged 3 commits into from
Jun 12, 2016
Merged

Conversation

hellais
Copy link
Member

@hellais hellais commented Jun 8, 2016

This implements one of the solutions outlined in #86 to support saving the state of a ooni-backend instance when it is shutdown.

Basically we have two files for an opened report with ID $REPORT_ID they both reside in the $REPORTS_DIR.

  1. The file at the path $REPORTS_DIR/$REPORT_ID is the actual partial report, that get's updated every time we need to create a new entry.
  2. The file at the path $REPORTS_DIR/$REPORT_ID-metadata.json contains the metadata associated with this report (the metadata sent on open()).

Every time we get a new update request we append to file 1. and touch file 2.

At regular intervals we look inside of the $REPORTS_DIR for all files ending with -metadata.json and look at their mtime with stat. If they have not been modified in a time frame greater than the stale_time we consider them stale and close them.

* Instead of having the report IDs in memory we have two files in the
  temporary raw_reports directory. One named like the report ID is the
  actual reports. Another is instead a file containing the metadata for
  the report.

This implements one of the proposed solutions outlined in #86
…cking

* We consider a report to be stale if current_time - modification time
  of report metadata is > than stale_time
@hellais hellais added the enhancement improving existing code or new feature label Jun 8, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 61.438% when pulling 89e2e46 on feature/fs-db into 6bd4a24 on master.

@bassosimone
Copy link
Member

Epic! Will review and test this!

from oonib.deck.api import deckAPI
from oonib.report.api import reportAPI
from oonib.input.api import inputAPI
from oonib.policy.api import policyAPI
from oonib.bouncer.api import bouncerAPI
Copy link
Member

Choose a reason for hiding this comment

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

For symmetry with moving APIs in class scope you would probably want to move also this statement

@bassosimone
Copy link
Member

bassosimone commented Jun 10, 2016

  • diff looks good
  • testing using MK as input
  • testing using MK instrumented not to close the report
  • testing using old YAML format
  • shutdown and restart of oonib while ooniprobe is running a deck
  • check that stale reports are pruned
  • some more testing
  • merge

The test where I shut down and then restarted oonib while ooniprobe was running a deck was quite interesting. I lost some entries, of course, but after oonib came up again, everything continued to work and subsequent entries were appended to the report file.


As regards testing using the old YAML format, I checked out v1.3.2 and run the following command (the same I used for testing restart of backend while ooni v1.5.0dev deck was in progress):

./bin/ooniprobe-dev -c http://127.0.0.1:10081 -i deck-it/0.1.0-it-user.deck

What I obtain is this error and of course no report is created:

[!] Failed to connect to reporter backend
Traceback (most recent call last):
  File "/Users/sbasso/src/TheTorProject/ooni-probe/ooni/reporter.py", line 333, in createReport
    response = yield self.agent.request("POST", url,
AttributeError: 'OONIBReporter' object has no attribute 'agent'

@hellais, any hint?

@hellais
Copy link
Member Author

hellais commented Jun 10, 2016

I believe the issue lies in the fact that ooniprobe 1.3.2 doesn't support non httpo collectors (i.e. https or http). I would advise you run it by specifying the collector address in httpo format.

* Implement a unittest for verifying that the stale report cleaning
  function works.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 62.204% when pulling 28caaaa on feature/fs-db into 6bd4a24 on master.

@bassosimone
Copy link
Member

I would advise you run it by specifying the collector address in httpo format.

Yes, this strategy is working, thanks!

@bassosimone
Copy link
Member

I've done more testing! Going to merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improving existing code or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants