obscuratus
Whew!
obscuratus
I think I sorted out the problems I'd been seeing in my segmented netDb branch.
obscuratus
The SAM tests are passing, and most of the other issues I'd been seeing in the logs have settled down.
dr|z3d
sounds promising, obscuratus
eyedeekay
Excellent to hear, I'll pull in the changes and look at them right away. Will be merging today
obscuratus
eyedeekay: OK, I haven't pushed the latest change yet.
obscuratus
It screams for a refactor, but it might be easier to review in its current form.
obscuratus
But I'm really happy with what I'm seeing in testing.
obscuratus
It's also a change that's worthy of review. There's a lot of intricacy in the way message handling is implemented. Migrating that intricacy to segmented netDb has been interesting. :)
eyedeekay
Yeah I've been following along, one very obvious conclusion is that we could not have done this without your help, thank you very much
eyedeekay
This was something I had been told at times was probably unrealistic because of the subtleties of how it was all plumbed in and you spotted a lot of stuff I missed
eyedeekay
It's an enormous step for us and I can't thank you enough for all your help
obscuratus
Thanks!
eyedeekay
All right I'm going to resolve my UI stuff on top of that, and then I think we're ready to put this on master
obscuratus
eyedeekay: I've pushed two commits to my i2p.i2p.2.4.0-test1-obscuratus-wip-20230822 branch.
obscuratus
One is a minor logging fix.
obscuratus
The other, NetDb: Add Processing for Client DSM, is the one that would need more review.
obscuratus
For reference, a lot of that is cut-and-paste from inNetMessagePool.
eyedeekay
OK I'll take a closer look
obscuratus
One part that could certainly use an extra set of eyes is when you're in the Tunnel InboundMessageDistributor, and you're setting something called a "ReplyJob".
obscuratus
On the other hand, this is stuff we've always been doing, because these messages were just re-routed back to the inNetMessagePool before.
obscuratus
Before this change, my router was having fits successfully completing a search. They would just time out, because the replies weren't getting matched up with the original search.
eyedeekay
I'm about 70% sure it's OK but I need to look at what OutNetMessage.getOnReplyJob is setting up to really know
obscuratus
As far as I could figure out, the ReplyJob is whatever was passed when the search was started. So it can vary from case-to-case.
obscuratus
It makes it really difficult to trace back.
eyedeekay
Yeah I see that. I'm looking for clues in all the implementors of ReplyJob now
eyedeekay
So we've got DirectLookupMatchJob, FloodOnlyLookupMatchJob, SearchUpdateReplyFoundJob as potential ReplyJobs
eyedeekay
DirectLookupMatchJob does not have it's own RunJob it gets it from Job so it does not actually run anything, call that "safe"
eyedeekay
SearchUpdateReplyFoundJob handles DSM's by routing them to the "outer" netdb(by calling netDb()) *but* by that time they should have been tagged with a dbid, so the outer netDb should automatically find the subDb, pretty sure that is also "safe"
eyedeekay
FloodOnlyLookupMatchJob is the one I am least sure about but I'm prepared to go with these changes
obscuratus
One thing that kind-of stands out as a *potentially* remaining missing item is DatabaseLookupMessage handling.
obscuratus
The InboundMessageDistributor doesn't even seem to have handling for DLM, so it hasn't come up.
obscuratus
It's probably fine, but I've just been noticing that it's a message that hasn't come up on my radar as needing attention, so I haven't really given it any. :)