search

atom / telemetry

sends usage metrics to GitHub's internal analytics pipeline
MIT License
11 stars 12 forks source link

Fix race condition when sending daily stats (take 2) #29

Closed rafeca closed 5 years ago

rafeca commented 5 years ago

Summary

This PR adds a simple locking system using the localStorage mechanism to prevent two different instances of telemetry (which could run in parallel from different windows/tabs in a browser/electron environment).

This, combined with https://github.com/atom/telemetry/pull/28, should solve most concurrency issues on the telemetry package.

Remaining issue

There's still one remaining potential issue which is minor (at least compared to this one and the one fixed by https://github.com/atom/telemetry/pull/28):

Any event happening while the reporting is being done is lost: Since happens because we're clearing all the events once the current events are being reported.

In order to fix this, we have two options:

  1. Implement a queueing system which places the events that happen while previous events are being sent in a temporary structure and then stores them in the database once the events got sent successfully.
  2. Instead of deleting all events from the database once they are sent, only delete the ones that were actually sent (this could potentially be slow or harder to implement).

Anyways, we should fix this remaining issue as a separate PR since it's orthogonal to this work.