Re: patch 2/2 debuginfod server etc.

[Mark Wielaard <mark at klomp dot org>]

Hi,

On Fri, 2019-11-15 at 14:54 -0500, Frank Ch. Eigler wrote:
> > > +set -x
> > 
> > Is this really necessary?
> > It makes the log files very verbose.
> > Or is that the intention?
> 
> I've found it tricky to debug failing tests without this.

OK, but note that you can place echo "starting phase x..." in the test
and those will show up in the logs.

> > > +# find an unused port number
> > > +while true; do
> > > +    PORT1=`expr '(' $RANDOM % 1000 ')' + 9000`
> > > +    ss -atn | fgrep ":$PORT1" || break
> > > +done    
> > 
> > Which package does ss come from?
> 
> iproute-5.2.0-1.fc30.x86_64
> 
> > Make sure it is listed as a BuildRequires.
> 
> OK, because of %check in the RPM?

Yes, thanks.

> > > +env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS=
> > > ${abs_builddir}/../debuginfod/debuginfod -vvvv -d $DB \
> > > +-p $PORT1 -t0 -g0 R F &
> > > +PID1=$!
> > > +sleep 3
> > 
> > Is there no better way to test the server has started?
> 
> This one may not be needed.
> 
> > > +mv prog F
> > > +mv prog.debug F
> > > +kill -USR1 $PID1
> > > +sleep 3 # give enough time for scanning pass 
> > 
> > Hmm that hardcoded sleep 3 again :{
> 
> Well, here's the problem.  We want to assert that the scan was
> successful.  We can't just reject a 404 response because the scan may
> not have finished yet.  So we'd have to race/loop/poll.  But then we'd
> need a timeout (how long?).  It turns out to be the same thing, just
> more complicated.
> 
> 
> > > +mv testfile-debuginfod-0.rpm R
> > > +mv testfile-debuginfod-1.rpm R
> > > +mv testfile-debuginfod-2.rpm R
> > > +kill -USR1 $PID1
> > > +sleep 10
> > 
> > Why 10?
> 
> To give extra time for scanning RPMs.
> 
> > > +kill -USR1 $PID1  # two hits of SIGUSR1 may be needed to resolve .debug->dwz->srefs
> > > +sleep 10
> > 
> > And another :{
> 
> Yes, again, same reasons as above.  You can either have a timeout-poll
> loop, or a time sleep and a single judgement poll.

Now that we have the metrics maybe we can poll those to see if the new
files have been indexed?

The reason I am complaining about this, is that make check -j8 on my
system takes (without valgrind):

real	3m6.749s
user	0m42.627s
sys	0m31.588s

Of which 2m seem to just be sleeps in run-debuginfod-find.sh.

> > > +# Trigger a cache clean and run the tests again. The clients should be unable to
> > > +# find the target.
> > > +echo 0 > $DEBUGINFOD_CACHE_PATH/cache_clean_interval_s
> > > +echo 0 > $DEBUGINFOD_CACHE_PATH/max_unused_age_s
> > > +
> > > +testrun ${abs_builddir}/debuginfod_build_id_find -e F/prog 1
> > > +
> > > +testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $BUILDID2 && false || true
> > 
> > OK. But that means zero means never cache/always clean?
> > I would have expected 0 to mean "forever".
> 
> I see the man page doesn't specifically disclose the interpretation of
> zero.  A "no retention of prior results" purpose is useful, and is
> consistent with 0 as per the text.  A "retain forever" setting would
> have to be a different value.

Could you add the current interpretation of zero to the manual page?
We could have some other setting (-1?) for "forever", but that doesn't
seem urgent.

Cheers,

Mark