Closed kanongil closed 7 years ago
The code looks good and seems to work as advertised, but upon further review I am still pondering the implications of this change.
@devinivy What are your concerns?
I just wanted to make sure I fully understood what was going on, cause it requires a slightly subtle understanding of how the configuration options interact with each other. I thought about it more, and I think the fix is good 👍
I have thought about it quite a bit, and I'm happy that you agree. It is a bit convoluted and somewhat complicated by a partially broken pendingGenerateTimeout
implementation.
Points to ease comprehension:
Policy#get()
will queue calls while a Policy#_generate()
is processing for key
.Policy#_generate()
ever fails to call respond()
, no responses will be triggered.generateTimeout
is supposed to act as a safeguard when generateFunc
doesn't call the callback, I'll try to get to this early next week.
@hueniverse I hate to poke but this fixes a rather serious (though hard to trigger) bug that effects any hapi server that uses catbox caching. Hope you will find some time to look at it soon.
We are running into this as well, if our api server goes down the get
calls don't complete and just pile up, so our webserver's cpu spikes and will not go down unless we restart them.
Yes, this is definitely an outstanding bug– I still think the fix here is solid 👍
This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.
This fixes a critical issue where a generate that never completes can cause
get()
calls to queue up indefinitely even thoughgenerateTimeout
is specified.This issue is especially critical in combination with
pendingGenerateTimeout
, which will never callrespond()
if there is a pending generate.