Am 2022-05-14 20:01, schrieb Henrik K:
On Sat, May 14, 2022 at 07:15:45PM +0200, Michael Storz wrote:
Despite all these changes, however, I still see room for improvement.
At the
moment I'm not very happy about how the do_meta_tests subroutine is
implemented. It seems to work now, but the query for the dependency of
the
meta rules looks too complicated to me:
foreach my $r (@{$md->{$rulename}|[]}) {
next RULE unless exists $h->{$r} && !$tp->{$r} && !$pl{$r};
}
Instead of three dependencies, there should really only be one query
at this
point for "is the rule ready or not". If we knew exactly the state of
a rule
at each point in time, then it would also be possible to move from a
brute
force algorithm to a deterministic algorithm, where a rule that
becomes
ready automatically sets all meta rules to ready if that rule was the
last
dependency analogous to the algorithm for tags. do_meta_tests would
then
simply process a queue of ready meta rules. If a new meta rule is
ready it
will be added to the end of the queue. If the queue is empty, all
rules of a
priority class are processed.
I heartily agree with everything. The problem is that I don't have any
formal programming training or knowledge of fancy algorithms or
deterministic stuff. Frankly I'm not even interested in most of that
stuff,
I just script and try to make things do stuff correctly with some
common
sense and logic, would be happy if someone showed how to make it
simpler.
Glad I was able to make it work with maybe 10% performance loss, but
it's
balanced back by dumb metas not needing to wait for async stuff for no
reason etc.
I'm glad you agree with my analysis. Then let's have one last try. I
think I found the logical error in the evaluation that makes everything
so complicated. The error already exists before 3.4.6.
Let's recap:
At the beginning of the evaluation, ALL rules are in the pending state
by definition. When rules are run, they change from state pending to
state finished. How is this state change accomplished?
- The normal case is the indirect change for the synchronous standard
rules. After they are run, they return either 0, which means it was a
miss, or a positive integer, which means it was a hit.
- Then there is the case of synchronous rules with explicit state
change. These rules call got_hit for a hit and should call got_miss if
it was a miss. got_miss is similar to rule_ready, it is just the
statement
$self->{tests_already_hit}->{$rule} ||= 0;
BTW, a better name for tests_already_hit would be
tests_already_finished or tests_finished.
- And then we have the asynchronous rules, which must be divided into
two parts: the synchronous part and the asynchronous part. The
synchronous part is the call of the eval function, which in turn submits
the asynchronous part to a queue function. At the end, the synchronous
part is supposed to return a result. And here comes the error: it
returns the value 0, which means that the rule is finished and the
result is a miss.
THIS IS WRONG.
The rule is not yet finished and the result is not yet known. Therefore,
it must NOT return 0, it must return undef. This means that it is still
pending (no state change) and the asynchronous part will decide if it is
a hit or a miss.
Now, let's test a correction:
- First, add a debug statement to the foreach loop of sub do_meta_tests
to see the values of the three conditions. The idea is to find out how
many and which rules depend not only on $h->{$deprule}, but also on
$tp->{$deprule} and $pl{$deprule}. If there are none left, we can
eliminate the two conditions.
The states of the three conditions are
result $h $tp $pl
OK 0 0 0
OK 0 1 0
OK 0 0 1
OK 0 1 1
OK 1 0 0
NO 1 1 0
NO 1 0 1
NO 1 1 1
NO is the state we do not want to have: there is a hit/the rule is
finished, but $tp and/or $pl are true which means the rule has not yet
finished.
# Meta is not ready if some dependency has not run yet
if (exists $h->{$deprule} && ($tp->{$deprule} || $pl{$deprule})) {
dbg("rules: NO: \$h exists, \tp=" . ($tp->{$deprule} ? 'true' :
'false') . "\$pl=" . ($pl->{$deprule} ? 'true' : 'false'));
} else {
dbg("rules: OK: \$h" . (exists $h->{$deprule} ? '' : ' not') . "
exists, \tp=" .
($tp->{$deprule} ? 'true' : 'false') . "\$pl=" .
($pl->{$deprule} ? 'true' : 'false'));
}
foreach my $deprule (@{$md->{$rulename}||[]}) {
if (!exists $h->{$deprule} || $tp->{$deprule} || $pl{$deprule}) {
next RULE;
}
}
Then run a test and see how many NO we get.
- Second, change sub run_eval_tests
in
# Make sure rule is marked ready for meta rules using $hitsptr
$evalstr .= '
if ($scoresptr->{q{'.$rulename.'}}) {
$hitsptr->{q{'.$rulename.'}} ||= 0;
$rulename = q#'.$rulename.'#;
delete $hitsptr->{q{'.$rulename.'}} ||= 0;
change
$evalstr .= '
if ($result) {
$self->got_hit($rulename, $prepend2desc, ruletype => "eval",
value => $result);
'.$dbgstr.'
}
}
to
$evalstr .= '
if (defined $result) {
if ($result) {
$self->got_hit($rulename, $prepend2desc, ruletype => "eval",
value => $result);
'.$dbgstr.'
} else { # got a miss, if not set by an explizit got_hit
$hitsptr->{q{'.$rulename.'}} ||= 0;
}
}
- Third, change all registered eval functions in DNSEval.pm to return
undef instead of 0
- Then make a new test and check if the eval functions have disappeared
from the debug statements with NO. If this is the case, the error is
correctly identified.
Regards,
Michael
