search

krux / starport

Apache License 2.0
2 stars 7 forks source link

Switching backend to akka #61

Closed realstraw closed 4 years ago

nburke-salesforce commented 4 years ago

@realstraw it would also help to understand why the change is being made -- maybe a note in the description, at least, since we're not addressing a known issue, and high-throughput message handling isn't a concern. Why now?

realstraw commented 4 years ago

@nburke-salesforce there are actually many issues I discovered when I started to look closer to the implementation https://github.com/krux/starport/pull/57 and start testing it. To name a few:

  1. Pipelines that are failed scheduling are no longer retried properly, where I had applied temporary fix here: https://github.com/krux/starport/pull/60/files#diff-27d434d84be8dde0057ec461ea3b9681R100-R109
  2. https://github.com/krux/starport/pull/57 breaks individual pipeline scheduling's atomicity. If some issue happened in starport activity and failed before retrieve is called, all the pipeline it tries to schedule would be double scheduled, which can be very problematic.

There are other issues I discovered too making this very hard to maintain. My choices are either roll back or move to a better implementation (which we were originally planned to use, but for various reasons did not go for while adding support for alternative (e.g. SQS) dispatchers.

realstraw commented 4 years ago

there is also additional benefit of making new type of (remote) dispatchers much safer, and even enables the ability to have mixed types too (the previous implementation would be either this or that at the starport level).

nburke-salesforce commented 4 years ago

@realstraw understood! Thank you for the additional context