search

vapor / redis

Vapor provider for RediStack
MIT License
458 stars 57 forks source link

Remove currentSend check to let the requests get enqueued #135

Closed vzsg closed 5 years ago

vzsg commented 5 years ago

Fixes #111. I'm not 100% certain that this is universally correct, but tests still check out, including a new test that previously crashed.

Sidenote, it seems RedisTests.testPubSubSingleChannel is not deterministic.

tanner0101 commented 5 years ago

Hmm actually we are seeing a failure:

/root/project/Tests/RedisTests/RedisTests.swift:59: error: RedisTests.testPubSubSingleChannel : Asynchronous wait failed - Exceeded timeout of 2.0 seconds, with unfulfilled expectations: Subscriber should receive message
tanner0101 commented 5 years ago

If we can resolve the test failures, let's re-open. However, I think the issues may be fundamental to QueueHandler's design. See more info in #133 regarding @Mordil's attempted backport of the new channel handler design.

vzsg commented 5 years ago

To be honest, I don't know what QueueHandler does. On the tin, it says that it should support queueing up multiple requests (which Redis implicitly supports, considering it's a single-threaded server), but the effect seems to be lost, and that has been boggling me for months.

This PR was not meant to be a serious fix, there's no real effort behind it. Let's keep this closed, and hope that even Vapor 3 users can see the light at the end of the tunnel.