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

Make checkinterval for monitor configurable #31

Closed
Alexuy opened this issue Jun 22, 2018 · 24 comments
Closed

Make checkinterval for monitor configurable #31

Alexuy opened this issue Jun 22, 2018 · 24 comments

Comments

@Alexuy
Copy link

Alexuy commented Jun 22, 2018

Can you add checkInterval property to config file or command line arg?

Now it's hardcoded in main.d:

immutable auto checkInterval = dur!"seconds"(45);
@abraunegg
Copy link
Owner

@Alexuy, @Abasz, @gitbls

Can you please help validate PR #97 ?

git clone https://github.com/abraunegg/onedrive.git
cd onedrive
git fetch origin pull/97/head:pr97
git checkout pr97
make
make install

Once built, create a config file:

vi ~/.config/onedrive/config 

With the following:

monitor_interval="300"

The monitor process should now operate at whatever numeric value is set.

@Abasz
Copy link

Abasz commented Aug 3, 2018

Hi,

will do shortly. Is monitor interval in milliseconds, seconds or minutes?

@gitbls
Copy link

gitbls commented Aug 3, 2018 via email

@Abasz
Copy link

Abasz commented Aug 3, 2018

So, I tested this and although interval work there is a slight problem with this line:
if (currTime - lastCheckTime > checkInterval) { in main.d

For instance, my library is rather big, so one monitor loop runs about 30 minutes. So if I set the checkInterval less than that it becomes continuous (apart from the first loop start where the interval is kept) since the above is always satisfied and check interval time is not honoured.

@gitbls
Copy link

gitbls commented Aug 3, 2018 via email

@abraunegg
Copy link
Owner

abraunegg commented Aug 3, 2018

@gitbls

On startup, note the monitor interval in syslog (along with the other startup messages), for completeness and peace of mind.

Will do - that is an easy one to add in & makes sense

Also, syslog had the message “The item database is incompatible, re-creating database table structures”. I assume that this is OK, but noting it for completeness.

Yes this is entirely normal due to providing a resolution for ensuring all database data is correct due to prior skilion issues.

@Abasz

So, I tested this and although interval work there is a slight problem with this line:
if (currTime - lastCheckTime > checkInterval) { in main.d

For instance, my library is rather big, so one monitor loop runs about 30 minutes. So if I set the checkInterval less than that it becomes continuous (apart from the first loop start where the interval is kept) since the above is always satisfied and check interval time is not honoured.

Thanks for the feedback / use case. Will look at this logic & see how best to improve.

@Abasz
Copy link

Abasz commented Aug 3, 2018

I think lastCheckTime should be set to currTime at the end of the monitor loop rather than at the beginning and it should be sorted.

@abraunegg
Copy link
Owner

agreed .. it is being set in the wrong place

@abraunegg
Copy link
Owner

Submitted a new commit in the PR - 7a864a2

Can you re-pull / retest using the latest commit in the PR? This should fix:

  • Update lastCheckTime logic
  • Add in log line for monitor mode interval

@gitbls
Copy link

gitbls commented Aug 3, 2018 via email

@abraunegg
Copy link
Owner

abraunegg commented Aug 3, 2018

What is the command line your using?

The log line will only echo out locally when using verbose mode or you can check the user activity log in the following locations:

  • ~/onedrive.log
  • /var/log/onedrive/%username%.onedrive.log

Console:

[root@localhost onedrive]# ./onedrive --monitor --verbose         
Loading config ...
Using Config Dir: /root/.config/onedrive
Initializing the OneDrive API ...
Opening the item database ...
All operations will be performed in: /root/OneDrive
Initializing the Synchronization Engine ...
Account Type: personal
Default Drive ID: <redacted>
Default Root ID: <redacted>
Remaining Free Space: 5364645376
Fetching details for OneDrive Root
OneDrive Root exists in the database
Initializing monitor ...
OneDrive monitor interval (seconds): 15
Monitor directory: .

Logfile:

2018-Aug-04 07:07:19.674992 Using Config Dir: /root/.config/onedrive
2018-Aug-04 07:07:19.686821 Initializing the OneDrive API ...
2018-Aug-04 07:07:21.7974046 Opening the item database ...
2018-Aug-04 07:07:21.8610772 All operations will be performed in: /root/OneDrive
2018-Aug-04 07:07:21.8819378 Initializing the Synchronization Engine ...
2018-Aug-04 07:07:23.8652664 Account Type: personal
2018-Aug-04 07:07:23.8653888 Default Drive ID: <redacted>
2018-Aug-04 07:07:23.8654553 Default Root ID: <redacted>
2018-Aug-04 07:07:23.8655174 Remaining Free Space: 5364645376
2018-Aug-04 07:07:23.8655807 Fetching details for OneDrive Root
2018-Aug-04 07:07:24.1105428 OneDrive Root exists in the database
2018-Aug-04 07:07:24.1106802 Initializing monitor ...
2018-Aug-04 07:07:24.1107609 OneDrive monitor interval (seconds): 15
2018-Aug-04 07:07:24.1108714 Monitor directory: .

@gitbls
Copy link

gitbls commented Aug 3, 2018 via email

@abraunegg
Copy link
Owner

OK .. the onedrive.service file does not include the --verbose tag which would then present this information to you - this is why your not seeing this in your logs.

@gitbls
Copy link

gitbls commented Aug 3, 2018 via email

@abraunegg
Copy link
Owner

Yes this is easy to do - its a balancing act as to what level of information is appropriate for normal logging vs verbose logging.

Pushed fc0c358 to PR97 to drop the logging level requirement.

Can you please recheck?

@gitbls
Copy link

gitbls commented Aug 3, 2018 via email

@abraunegg
Copy link
Owner

One other suggestion: Since the skip_file argument can be a bit “interesting”, it might be good to output that to the log on startup as well.

There is work going on to totally overhaul all the logging - from normal, verbose through to debug

In fact, why not go all the way and output sync_dir (and any other current or future config items as well).

Agree in principal here, however for 'normal' log level no, for increased verbosity yes. Normal log level should be minimal + any errors, not entire configuration parameters being used at startup.

A better option here is potentially a new flag like --display-operational-config where it displays the configuration which would be used if a sync was to occur. This way you can check the configuration without syncing.

@gitbls
Copy link

gitbls commented Aug 3, 2018 via email

@Abasz
Copy link

Abasz commented Aug 4, 2018

Although I do not what is wrong yet but the interval does not seem to be kept. Below is the log (I added some log entries manually to monitor the loop). Loop is set to 5min. Before the first loop timeout is kept. But later the next loop starts immediately

2018-Aug-04 12:20:16.418354 Initializing the Synchronization Engine ...
2018-Aug-04 12:25:29.3048253 starting loop: MonoTime(960945176290649 ticks, 1000000000 ticks per second)

2018-Aug-04 12:25:29.5627217 Syncing changes from OneDrive ...
2018-Aug-04 13:02:43.6050343 loop finnished: MonoTime(960945176290649 ticks, 1000000000 ticks per second)
2018-Aug-04 13:02:43.6468427 starting loop: MonoTime(963179518270076 ticks, 1000000000 ticks per second)
2018-Aug-04 13:02:44.1995263 Syncing changes from OneDrive ...
2018-Aug-04 13:39:54.3035005 loop finnished: MonoTime(963179518270076 ticks, 1000000000 ticks per second)
2018-Aug-04 13:39:54.3332532 starting loop: MonoTime(965410204727828 ticks, 1000000000 ticks per second)
2018-Aug-04 13:39:54.8907423 Syncing changes from OneDrive ...

@abraunegg
Copy link
Owner

Thanks for the feedback - I see where it is going wrong at the moment - will post an update shortly.

@abraunegg
Copy link
Owner

@Abasz
Posted 7dc8dc5 which should resolve this issue your seeing - can you please recheck this PR ?

@Abasz
Copy link

Abasz commented Aug 4, 2018

Confirmed, this works nicely. good work :)

abraunegg added a commit that referenced this issue Aug 7, 2018
…e (Issue #31) (#97)

* Implement Feature: Make checkinterval for monitor configurable
* Update monitor mode processing logic
@abraunegg
Copy link
Owner

Closed due to PR #97 merged

@lock
Copy link

lock bot commented Jan 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Jan 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants