dpol1 commented on PR #2052: URL: https://github.com/apache/stormcrawler/pull/2052#issuecomment-5490244845
Agree with Julien's open points; three things I noticed on top while reading the store. `promoteIfEstablished` flips the flag without taking the lock `discardWeakest` holds — couldn't a param get promoted right as the other thread picks it as weakest? That would withdraw a rule after it became visible, which is the flip-flop the comment above `discardWeakest` says can't happen. Am I missing a barrier somewhere? On the pagination guard: wouldn't `/shoes?pg_no=2`, `/shirts?pg_no=2`, `/hats?pg_no=2` each canonicalising to page 1 count as three distinct paths and pass `min.distinct.paths`? If so the protected-name list ends up as the only defence, and it matches case-sensitively and in English only — `Page`, `pageNo`, `pagina` slip through. Maybe case-insensitive matching, and soften the "confined to a single path" sentence in internals.adoc? Last one on the docs: "briefly disagree / converge" — a worker with no parser executor never learns, so it diverges for good rather than briefly. Worth spelling out so people size parser parallelism with that in mind? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
