Madelyn Olson
Madelyn Olson
> btw is there any reasoning behind the requirement for the signoff? Technically we adopted it because it's an LF requirement, but it's also a good practice to force a...
@PingXie Sorry, I saw your approval and had intended to merge this on Friday but ran out of time and missed that you had comments. They seemed like small comments,...
Since we don't automatically test on FreeBSD: https://github.com/valkey-io/valkey/actions/runs/8994018097
> Thanks, I see it's succeeded. Just out of curiosity, why didn't we set up a macOS runner that would run by default? Runner execution time was the historical reason,...
@panjf2000 It makes sense to me, I just haven't had a sense to walk through it and convince myself the code is correct. Hopefully tomorrow?
> which should have a significant benefit to the performance. Would you mind using `valkey-benchmark -r 10000000 -d 100 -n 10000000` with and without to change to compare the performance?...
Ok, those performance numbers are compelling, although I'm a little surprised at the result. During the typical command flow, we shouldn't call any of these functions, so I'm surprised to...
Ok, so I'm still worried about the panic and how it might impact production systems for edge cases related to kqueue failures. I would like to get the performance improvement,...
@PingXie Related to your comment about event handling, I think we should look more seriously into making sure that `connSet(Read|Write)Handler` always succeed. Once that is done, we can update the...
> To make it clearer, by "update the kqueue batch handler", you meant removing the USE_KQUEUE_BATCH macro in this PR and make it the only code path, right? Yeah, this...