[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #70 from Henrik Krohns --- I think Bill summed it up quite nicely. There's some agreement happening, so I'll start tuning things to the way of old logic. Nothing is going to waste, as the new code will still help make metas run in order without pesky priorities. Other than that, Kevin, Loren, please read the bug through again with thought, and if there's still something unclear, you can try asking. But I really don't have extra time to go over it all again as the details should be all there (admittedly scattered). -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 Bill Cole changed: What|Removed |Added CC||billc...@apache.org --- Comment #69 from Bill Cole --- (In reply to Henrik Krohns from comment #62) > Ok so all pending commits are now done. > > This bug is still open for: > > - Discussion about what to do with "unrun" rules > - Documentation > > Please vote or discuss: > > - Keep logic as is, unrun rules may prevent many metas from running > - Revert to old logic, unrun rules are evaluated as not hitting (0) > > I probably lean a bit to the old logic. It doesn't require much changes to > code, just some tweaking to last do_meta_rules/finish_meta_rules call. I lean very strongly to the old logic because: 1. It is certain that some meta rules in the wild rely on it by design and will have their performance changed if we use the new logic. The change won't necessarily be obvious to people who have deployed such rules. 2. I don't think we understand the changed logic well enough to get it right in a rush to a major release. 3. It limits the impact of disabling costly rules, because 'unrun' subrules don't create a tree of "don't run" dependencies. 4. The logic is more obvious to users. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 8068] t/spamd_ssl_accept_fail.t failure on slow systems
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8068 Sidney Markowitz changed: What|Removed |Added CC||sid...@sidney.com Status|NEW |RESOLVED Resolution|--- |FIXED Target Milestone|Undefined |4.0.0 --- Comment #1 from Sidney Markowitz --- trunk % svn ci -m "bug 8068 - Add a delay in the test to allow for some slow test systems" t/spamd_ssl_accept_fail.t Sendingt/spamd_ssl_accept_fail.t Transmitting file data .done Committing transaction... Committed revision 1904818. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 8068] New: t/spamd_ssl_accept_fail.t failure on slow systems
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8068 Bug ID: 8068 Summary: t/spamd_ssl_accept_fail.t failure on slow systems Product: Spamassassin Version: 4.0.0 Hardware: PC OS: Linux Status: NEW Severity: minor Priority: P2 Component: Regression Tests Assignee: dev@spamassassin.apache.org Reporter: sid...@sidney.com Target Milestone: Undefined t/spamd_ssl_accept_failt.t was written to test the fix for bug 4107 which was spamd crashing when it was launched with the option to accept only ssl connections and spamc called it using non-ssl protocol. The test consists of launching spamd with ssl option, calling it once with spamc without ssl, then again with spamc using ssl, then stopping spamd, confirming that the combined return from the two calls to spamc contains the expected spam report from the second call. In setting up testing on GitHub Actions, the test fails with the second spamc call not appearing to contact spmd, same as the non-ssl call. If I add a sleep(1) between the first and second call of spamc, the test works. I suspect that the fix for bug 4107 does not prevent the child process from crashing when it receives the non-ssl call from spamc. That is good enough as far as the fix goes, because the child gets re-spawned. If I'm right about the cause of the test failure, it is only a problem if spamd is running using ssl on a slower than useful for production system and spamc calls it without ssl from a machine that has access to the spamd port and then a proper ssl call from spamc happens less than a second later, and then the only result is that the second spamc call fails. If that is the only failure scenario I think it would be fine to just add a sleep(1) to the test so that it can pass on slow systems like the GitHub action runner. If anyone wants to look at the code and and make a more robust fix for bug 4107 that keeps the child from crashing, feel free. Since my proposed change is only in the test, I think I can commit it for 4.0.0 without a vote. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 Loren Wilton changed: What|Removed |Added CC|lwil...@earthlink.net | -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #68 from Loren Wilton --- (In reply to Kevin A. McGrail from comment #66) > So I believe the new logic is trying to be efficient and not run rules that > are unnecessary. I believe the change was done more for proper rule evaluation ordering, delaying dependent meta rules until all dependencies are available. If some of the dependencies are net rules that time out or are (I think) undefined because they are disabled we get to the case of having unrun meta rules waiting for results that will never be available, so the meta is also marked unrun. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #67 from Loren Wilton --- (In reply to Henrik Krohns from comment #65) > Not sure I follow, would it matter why a rule is unrun? People have been talking about meta rules that evaluate to something useful, and then "season" the result with a term from something like a net rule, that might not be run. If I understand them correctly, they are saying that they would like the result of that rule to be the result of it's expression, with the unrun rule zero. The result of this meta may not be zero, so the rule will end up scoring. If this meta were just set to zero because it was unrun, it likely would not produce the desired result. >From your comment #14: > Let's take this stock rule as example. > meta DIGEST_MULTIPLE RAZOR2_CHECK + DCC_CHECK + PYZOR_CHECK > 1 > This gets problematic even when considering disabled rules. If simply one of > these is not used/loaded, or lookup times out, it won't hit even if two of > them hit. So I claim that if razor and dcc run, but pyzor doesn't, the result should be: meta DIGEST_MULTIPLE RAZOR2_CHECK + DCC_CHECK + 0 > 1 Obviously the rule could still hit. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 Kevin A. McGrail changed: What|Removed |Added CC||kmcgr...@apache.org --- Comment #66 from Kevin A. McGrail --- So I believe the new logic is trying to be efficient and not run rules that are unnecessary. However, with meta rules, I think the efficiency ended up not running rules we should have. Is this correct, Henrik? If so, the old logic is a good triage while we try and revisit the efficiency logic. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #65 from Henrik Krohns --- (In reply to Loren Wilton from comment #64) > Can you tell the difference in the unrun rules list between a rule FOO that > didn't run because it was a net test that timed out (or is undefined, or > similar), and a rule BAR that is unrun because it depends on FOO? If so, I'd > say to zero the FOO result and then evaluate BAR. This is more complex than > simply marking all finally-unrun rules with a zero value, but is I think the > desired result. Not sure I follow, would it matter why a rule is unrun? -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 Loren Wilton changed: What|Removed |Added CC||lwil...@earthlink.net --- Comment #64 from Loren Wilton --- Can you tell the difference in the unrun rules list between a rule FOO that didn't run because it was a net test that timed out (or is undefined, or similar), and a rule BAR that is unrun because it depends on FOO? If so, I'd say to zero the FOO result and then evaluate BAR. This is more complex than simply marking all finally-unrun rules with a zero value, but is I think the desired result. I suppose there is a pathological problem of two rules that depend on each other. That could be resolved by arbitrarily marking one rule as zero and then running the other. Maybe that is already detected earlier and both rules are discarded with a lint error. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #63 from Wolfgang Breyha --- I vote for the old logic. The new one is unpredictable and makes maintenance of "homebrew rules" a burden. And in most cases they are used with "||" or "+" where a zeroed unrun rule makes no difference at all. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 7735] Meta rules need to handle missing/unrun dependencies
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=7735 --- Comment #62 from Henrik Krohns --- Ok so all pending commits are now done. This bug is still open for: - Discussion about what to do with "unrun" rules - Documentation Please vote or discuss: - Keep logic as is, unrun rules may prevent many metas from running - Revert to old logic, unrun rules are evaluated as not hitting (0) I probably lean a bit to the old logic. It doesn't require much changes to code, just some tweaking to last do_meta_rules/finish_meta_rules call. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 8062] [review] no URL makes uridnsbl rules "unrun"
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8062 Henrik Krohns changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #6 from Henrik Krohns --- Sendingtrunk/lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm Transmitting file data .done Committing transaction... Committed revision 1904811. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 8062] [review] no URL makes uridnsbl rules "unrun"
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8062 Giovanni Bechis changed: What|Removed |Added CC||giova...@paclan.it --- Comment #5 from Giovanni Bechis --- +1 to commit -- You are receiving this mail because: You are the assignee for the bug.
[Bug 8062] [review] no URL makes uridnsbl rules "unrun"
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8062 Kevin A. McGrail changed: What|Removed |Added CC||kmcgr...@apache.org --- Comment #4 from Kevin A. McGrail --- +1 from me as well, ty this should close 7736 as well, I believe. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 8061] [review] Fix meta handling for $suppl_attrib->{rule_hits}
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8061 Henrik Krohns changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #11 from Henrik Krohns --- Sendinglib/Mail/SpamAssassin/Conf/Parser.pm Sendinglib/Mail/SpamAssassin/Plugin/Check.pm Transmitting file data ..done Committing transaction... Committed revision 1904797. -- You are receiving this mail because: You are the assignee for the bug.
[Bug 8061] [review] Fix meta handling for $suppl_attrib->{rule_hits}
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8061 Giovanni Bechis changed: What|Removed |Added CC||giova...@paclan.it --- Comment #10 from Giovanni Bechis --- +1 to commit latest patch -- You are receiving this mail because: You are the assignee for the bug.
