Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#14826 closed enhancement (fixed)

backend: hourly update without crontab or the like

Reported by: iwakeh Owned by:
Priority: Medium Milestone: Onionoo 3.1-1.0.0
Component: Metrics/Onionoo Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Since I didn't want to bother with crontab,
I've been using the attached patch for a while.

If no property is given the backend update will run one time.
When -Donionoo.cron.runonce=false is given the updater will
run after waiting a minute and then every hour after a successful
update run. (Thus, it might also be a solution for #13003.)

Please review.

Child Tickets

Attachments (2)

Change History (12)

comment:1 Changed 5 years ago by iwakeh

Status: newneeds_review

comment:2 Changed 5 years ago by karsten

Status: needs_reviewneeds_revision

That patch starts the first execution 1 minute after starting and then waits for 60 minutes between termination of one execution to start of the next.

Can we try to get closer to the current crontab behavior? That would be start at 15 minutes after the hour, unless an earlier execution is still running. I think scheduleAtFixedRate does that, right? And I think we don't need the lock file anymore if we go that route, correct?

comment:3 Changed 5 years ago by iwakeh

Why should the crontab behavior be mimicked so closely?

Fixed rate scheduling would change the repeated update times also,
if a run takes longer than the set interval. Depending on the configuration
this might be another interval or an immediate restart.

The successive update-start depends only on the start time of the java process.
Started at 00:14 it would begin at 00:15. But I do not mind setting it to zero.

Especially, when there are several mirrors it might be advisable not to
do the downloads at the same time. The runtime of each run would spread the downloading over time.

Should the update interval be shorter, in order to have an update at about every hour?

The lock file will still be needed for the single run using crontab for scheduling
the executions. So, it should only show up there, I think.

comment:4 Changed 5 years ago by karsten

The idea to run around :15 of the hour is that we can be quite confident that new data will be available at that time. The consensus gets published at :00, and CollecTor starts fetching the consensus and other descriptors at :05. That process usually takes a few minutes. If we have Onionoo download new data from CollecTor at :15 or :20, it's going to be new data, and the earlier we process new data the fresher Onionoo's data will be.

You have a point about not having all mirrors start at the exact same time. What we could do is start between :15 and :20, depending on when the program starts. For example, if the program starts at :01, we start the first run at :16 and the next run 60 minutes later.

Note that we also must avoid running more than once per hour. There's a bug where if we don't parse a fresh consensus, the exit/guard/middle probabilities cannot be calculated correctly. If we plan to run more often, we'll have to fix that bug first. I could imagine running every five minutes and stopping a run if we figure out that CollecTor has no new data for us.

Thoughts? Thanks for starting this!

comment:5 in reply to:  4 Changed 5 years ago by iwakeh

Thanks for the explanation!

I attached a patch;

  • that will start the updater according to the 'onionoo.cron.runonce' property as above (default is the single run).
  • the update will be started between (incl.) :15 and :19 depending on start time
  • then it will run at a fixed rate of 60 minutes.

Is there a track-task for the incorrect probability calculation bug?

Last edited 5 years ago by iwakeh (previous) (diff)

comment:6 Changed 5 years ago by iwakeh

Status: needs_revisionneeds_review

comment:7 Changed 5 years ago by karsten

Please take a look at my branch task-14826 which contains your patch plus whitespace fixes. If you still like it, I'll merge that to master.

I'll look if there's already a ticket for the mentioned bug. If there's one, I'll cc you, and if not, I'll create one.

comment:8 Changed 5 years ago by iwakeh

The branch is fine; good to remove the redundant %60-operation.

comment:9 Changed 5 years ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Okay, great. Merged to master. Closing. Thanks!

comment:10 Changed 3 years ago by iwakeh

Milestone: Onionoo 3.1-1.0.0
Severity: Normal
Note: See TracTickets for help on using tickets.