huboard / huboard-web

GitHub issues made awesome
https://huboard.com
61 stars 26 forks source link

Creates a cached job to prevent event race conditions #304

Closed discorick closed 8 years ago

discorick commented 8 years ago

Alright, after much spiking and testing today - I think this should do the trick (I want to test it out further on the review app)

Unfortunately only production is fast enough to generate the race conditions, so we won't know for sure until we try there.

@rauhryan @dahlbyk can you poke holes here?

fixes https://github.com/huboard/huboard/issues/608

dahlbyk commented 8 years ago

The seemingly consistent duplicate New Issue notifications are really due to a race condition?

discorick commented 8 years ago

It really feels that way, if two workers are handling the GH webhook, plus HB event simultaneously there is potential for the cache (which is non-blocking) to be too late while its setting willPublish

I was not actually able to repro duplicates on local or huboard-rails - only on cedar which is the only ENV with blazing fast dedicated workers.

dahlbyk commented 8 years ago

Code change looks solid. More room to DRY up the two base event jobs, but that can wait. Go ahead and :shipit: if you're satisfied with your testing.

dahlbyk commented 8 years ago

I'm still getting a double-tap on the demo app... Is that expected?

discorick commented 8 years ago

Yeah, huboard-rails is picking up GitHub webhook, and since dalli is a local cache, the review app has the cache key, not huboard-rails.

rauhryan commented 8 years ago

possibly randomize the delay time? still hacky but it might help.

Also... could we restrict the trouble makers to a special queue that isn't parallel?

On Fri, May 20, 2016 at 3:23 PM, discorick notifications@github.com wrote:

Yeah, huboard-rails is picking up GitHub webhook, and since dalli is a local cache, the review app has the cache key, not huboard-rails.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/huboard/huboard-web/pull/304#issuecomment-220709367

dahlbyk commented 8 years ago

Switching the GH hook to also go through the review app resolved the double- tap. I think we're good!