logstash-plugins / logstash-output-google_cloud_storage

Apache License 2.0
9 stars 26 forks source link

Refactor queue to worker pool #29

Closed josephlewis42 closed 6 years ago

josephlewis42 commented 6 years ago

Change the old architecture of one upload thread that reads a queue of files to open to a new one where files to upload are pushed to a pool of workers only once they're ready to be uploaded.

This architecture simplifies things in a few ways:

The changes fix: #22, #5 and #15

Locking still isn't fixed, it's just slightly less bad than it was before. Once this PR and the Java one get merged there'll be a nice clean path towards locking.

josephlewis42 commented 6 years ago

Let me know if you want to talk through this one. There's a lot of stuff I'd like to fix in here but it's difficult because the plugin doesn't seem to have any reliable invariant.

At this point I'm more afraid of introducing deadlocks with locks than I am the crashes caused by race conditions, because at least Logstash will restart the plugin after a crash.

Two good next steps would be:

  1. Make a PR that fixes rubocop recommendations.
  2. Refactor the whole write/sync/rotate code into a class that has defined and testable invariants so it can have real locks.