-
Notifications
You must be signed in to change notification settings - Fork 3.3k
proxy: deduplicate protocol processing code #1206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: next
Are you sure you want to change the base?
Conversation
I added a lot of generic parsing code to mcmc while writing the inspector and mutator last year, which I've been able to use mostly without modification for this first big change. If I can get |
still want to revise the mcmc_ API's... but the plan is to probably keep using them by converting code and then update and re-test everything. |
192e82b
to
384a817
Compare
Trying to get the request/response struct's merged and fix some awkwardness with how mcmc's types/codes are handled. Taking more time than I hoped. |
trying to de-scope "single struct for req/res" from this PR. going to stick with:
then in separate PR's do more mcmc reworking... edit: maybe... or maybe just a longer series of commits in this change :/ trying to land with high test coverage but don't want the change to be too massive. |
cleaning up the ascii_multiget hack was time consuming but helps a shitton in reducing code fragility... |
aight got a few hacks cleaned up and did planning work for the big merge:
... then will re-evaluate if it's a good time to continue doing mcmc work or not. the mcmc response parser is still using strtol's too, which sucks. I kiiiinda feel like I could get something big out of finishing the mcmc work... since I could write some wrapper code and get an "instant mc-speaking server" out of it. which would allow simpler experiments with funny backends by just having them be proxy backends. |
6c46dea
to
a726074
Compare
got things wired up so I could convert one command at a time. still need another round of parser work after this stage though. All of the down to the last meta command for |
figuring out mset without duplicating code was a pain but I think it's close now. all of the base meta commands are ported. next is to port the base text protocol... get commands are going to be the annoying one since I'll need to write a wrapper. |
migrating get is moderately painful because of the multikey syntax. attempting to lift the recent hack cleanup code from proxy's version of this. |
all the storage commands are done. need to hype myself up for converting the maintenance commands to use the new tokenizer and an array/hash table instead of an if/else tree. then... delete the old tokenizer code and re-evaluate for what's next. |
31c6c53
to
d988902
Compare
All the commands are converted.
after that need to start on a few cleanup passes for some stuff that got dropped during the conversion (verbose output/etc) |
been having trouble getting time to sit down on this lately. just finished the asciiauth and the old tokenizer is now completely gone. input command buffers are now the next big step is a lot of small cleanups. then extended testing can begin. |
bc8e196
to
94729a1
Compare
sigh. wtf was I doing.... |
fixing the command length issue took way too long to unwind... looks great now though. |
updated the TODO list. |
maybe one more day to complete this? these are short days right now though :( |
oh right.. need to think about testing again.. |
simple this time.
need to check for an opportunity to reduce the storage_get callback sizes later.
execute via the new parser, else fall back to orig tokenizer. so I can migrate these one by one...
hopefully the rest go down easier...
forgot to commit after doing md. doh. mg feels slightly sketchy but looks right. also gives what might be mild binary key support for proxy when used with mcp.internal.
still some temporary code to clean up later, but this is the last meta command to move.
got stuck debugging a while. odd failure in udp.t removes an old workaround for an ancient form of the delete command.
easier since we get to reuse the pattern from mset.
abstracted multi-key parsing out of the actual get command code. now all the main parsing is finally shared.
shutdown, me, quit, version, stats, watch didn't want to do all that at once but meh.
needs another pass for min/max tokens and some conversion consistency.
... so we can use them.
was adding or removing 1 to the length randomly. fixed this and added some assertions to build confidence... which found a legitimate bug in mcp.request()
need to audit t->cur_sfd with mcp.internal.
this is the only place I could find it...
didn't think this would work but it does (since other parts are compiled out, it's safe)
would be nice to retire these soon...
was double-checking for noreply since we now grab it during the initial parsing. defaults to allowing noreply and maybe some functions should force-disable it?
This commit is step one of... a few... for standardizing the code paths for protocol handling. We started with three:
Both requests and responses can now be tokenized by the same code from mcmc. This commit series does this in stages, which will result in removal of duplicated code and fixing many long standing issues.
In this particular commit the duplicated tokenizer is removed from proxy_request.c, and mcmc functions are now used in various places.
All of the mcmc related functions have actual C unit tests behind it which should help them be more trustworthy.
GOALS:
proxy_internal.c
TODO:
This work is being done in stages because there's too much code to hold in my brain at once. Not necessarily in order, but:
t->cur_sfd
usage when combined with mcp.internalremove double-parsing request from proxy fallback code (maybe optional? this fallback code isn't really used)TESTING:
attempting to delay further cleanups:
meta_flag_preparse
. we have the bitflags now.Got another month or two of work on this including extra testing that should be done.