Use server time instead of local time in RWS (fixes #367)

[lucach]

Server time differs from local time in two cases (can be both):

• Different timezone
• Same timezone but clocks not aligned (for instance due to a misconfiguration of NTP)

[stefano-maggiolo]

I'll ask @lerks to review this, I'm not sure I can guarantee there is nothing else to be changed. In the meantime if you want you can apply my comment.

---

Review status: 0 of 5 files reviewed at latest revision, 1 unresolved discussion.

---

cmscommon/datetime.py, line 75 [r1] (raw file): With this change it would make more sense to have a signature like

get_timezone(contest, user=None)

---

Comments from the review on Reviewable.io

[lucach]

Review status: 0 of 7 files reviewed at latest revision, 1 unresolved discussion.

---

cmscommon/datetime.py, line 75 [r1] (raw file): Done (I did this way at first to minimize the changes).

---

Comments from the review on Reviewable.io

[lw]

I agree that these changes are useful and needed. Yet, first, as they are relatively independent I would ask you to split them into two different commits.

I don't like the way you handle "misaligned" clocks. You (ab)use the calls made to load data to update the offset estimate. There are a few issues I see with that. First, you patch an already messy code in many places, mixing unrelated functionalities. Then, you always update the offset using the latest estimate. If there is great variability in the network times this will cause the clock to constantly jump.

What you are doing is basically a very simplified NTP. I think a better way to implement it is by creating a dedicated module in the JS client, that performs cyclical independent requests to the server (to a dedicated URL), and keeps an offset estimate. To calculate this estimate we should use more of the techniques adopted by NTP: ignore "deviant" samples (too large ones), take a weighted average of the N most recent samples (giving more weight to the most recent ones), and gradually correct the offset instead of having it jump.

Or we could directly use one of the NTP implementations for JS. I haven't looked into them, though...

What do you think?

[lucach]

I agree with your comments. My first thought was that we shouldn't reinvent the wheel and we should use an existing JS library. However, after a quick look, I can say that there are no decent implementations and that they're barely maintained.

Anyway, we should not overcomplicate things: I think that the techniques you have outlined are more than enough for our goal.

[lucach]

I split the original commit into two separate commits (see lucach@40bb662 and lucach@36ad420). Timestamp requests are now independent from the rest of the code and point to a dedicated URL. Full NTP algorithms seems unreasonably complicated with respect to our goal. Moreover, the browser is already strained enough while it renders the page.

Current process works as following:

• Consider only latest ten requests
• Compute offset time and round trip time (RTT) of each request
• Compute the average offset time of the five shortest requests in terms of RTT
• Update the local clock if the difference with local time is greater than 10s, otherwise smooth the transition adding (or subtracting) at most 500ms.