public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
From: Mark Wielaard <mark@klomp.org>
To: Noah Sanci <nsanci@redhat.com>, elfutils-devel@sourceware.org
Subject: Re: [Bug debuginfod/28577] Make run-...-fd-prefetch-caches.sh test something
Date: Fri, 27 May 2022 18:20:47 +0200	[thread overview]
Message-ID: <ede645fbb9d97c1e7d2c97605e8bd684074289ab.camel@klomp.org> (raw)
In-Reply-To: <CAJXA7qhyJt+sGdp9zxGapxiOP95b+jz2cJC9NYBWyBw8SLHy5g@mail.gmail.com>

Hi Noah,

On Wed, 2022-05-25 at 14:44 -0400, Noah Sanci via Elfutils-devel wrote:
> PR28577: Make run-debuginfod-fd-prefetch-caches.sh test
>              something
> 
> Update to the run-debuginfod-fd-prefetch to make the test more sound.

Thanks. And according to Evgeny this makes the test pass for him:
https://sourceware.org/bugzilla/show_bug.cgi?id=29180#c2

But I find it a little hard to review, it would have been helpful to
get a bit more comments/documentation what exactly is being tested now.

> diff --git a/tests/run-debuginfod-fd-prefetch-caches.sh b/tests/run-
> debuginfod-fd-prefetch-caches.sh
> index 66a7a083..50ffc4ac 100755
> --- a/tests/run-debuginfod-fd-prefetch-caches.sh
> +++ b/tests/run-debuginfod-fd-prefetch-caches.sh
> @@ -21,22 +21,22 @@
>  # for test case debugging, uncomment:
>  set -x
>  unset VALGRIND_CMD
> +mkdir R Z

OK, some boilerplate.

> -mkdir R
> -cp -rvp ${abs_srcdir}/debuginfod-rpms R
> -if [ "$zstd" = "false" ]; then  # nuke the zstd fedora 31 ones
> -    rm -vrf R/debuginfod-rpms/fedora31
> -fi

This gets moved to below.

> -FDCACHE_MBS=$((1/1024/1024))
> +FDCACHE_MBS=1
>  FDCACHE_FDS=1
> -PREFETCH_MBS=$((1/1024/1024))
> -PREFETCH_FDS=2
> +PREFETCH_MBS=1
> +PREFETCH_FDS=1
>  PREFETCH=1

OK, new presets.

>  # This variable is essential and ensures no time-race for claiming
> ports occurs
>  # set base to a unique multiple of 100 not used in any other 'run-
> debuginfod-*' test
>  base=8800
>  get_ports
> +cp -rvp ${abs_srcdir}/debuginfod-rpms R
> +if [ "$zstd" = "false" ]; then  # nuke the zstd fedora 31 ones
> +    rm -vrf R/debuginfod-rpms/fedora31
> +fi
> +cp -rvp ${abs_srcdir}/debuginfod-tars Z

OK, put rpms under R, tars under Z.
 
>  DB=${PWD}/.debuginfod_tmp.sqlite
>  tempfiles $DB
> @@ -47,88 +47,68 @@ export DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache
>  ######
>  rm -rf $DEBUGINFOD_CACHE_PATH
>  rm -rf $DB
> +
>  # Set mb values to ensure the caches certainly have enough space
>  # To store the test files
>  env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod $VERBOSE -p $PORT1 -d $DB \
> -    --fdcache-fds=$FDCACHE_FDS --fdcache-prefetch-fds=$PREFETCH_FDS --fdcache-mintmp 0 -vvvvv -g 0 -t 0 -R R \
> -    --fdcache-mbs=50  --fdcache-prefetch-mbs=50 \
> -    --fdcache-prefetch=$PREFETCH  > vlog$PORT1 2>&1 &
> +    --fdcache-fds=$FDCACHE_FDS --fdcache-prefetch-fds=$PREFETCH_FDS  -vvvvv -g 0 -t 0 -R R \
> +    -Z .tar.bz2=bzcat Z  --fdcache-mbs=50  --fdcache-prefetch-mbs=100 \
> +    --fdcache-prefetch=$PREFETCH -F > vlog$PORT1 2>&1 &

Now run with --fdcache-fds=1 --fdcache-mbs=50 and
-fdcache-prefetch-fds=1 fdcache-prefetch=1 -fdcache-prefetch-mbs=100

It is these last 3 we want to test.

>  PID1=$!
>  tempfiles vlog$PORT1
>  errfiles vlog$PORT1
> +
>  # Server must become ready
>  wait_ready $PORT1 'ready' 1
>  ########################################################################
> -cp -rvp ${abs_srcdir}/debuginfod-rpms R
> -if [ "$zstd" = "false" ]; then  # nuke the zstd fedora 31 ones
> -    rm -vrf R/debuginfod-rpms/fedora31
> -fi
>  kill -USR1 $PID1
>  wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
> +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
> +wait_ready $PORT1 'thread_busy{role="scan"}' 0

OK, boilerplate to wait for initial scan.

>  # All rpms need to be in the index, except the dummy permission-000 one
>  rpms=$(find R -name \*rpm | grep -v nothing | wc -l)
>  wait_ready $PORT1 'scanned_files_total{source=".rpm archive"}' $rpms
> -kill -USR1 $PID1  # two hits of SIGUSR1 may be needed to resolve .debug->dwz->srefs
> -# Wait till both files are in the index and scan/index fully finished
> +kill -USR1 $PID1 
>  wait_ready $PORT1 'thread_work_total{role="traverse"}' 2
> -export DEBUGINFOD_URLS="http://127.0.0.1:"$PORT1
> +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
> +wait_ready $PORT1 'thread_busy{role="scan"}' 0

Do another scan and wait for it to be ready.

> -archive_hits=5
> -SHA=f4a1a8062be998ae93b8f1cd744a398c6de6dbb1
> -for i in $archive_hits
> -do
> -  archive_test c36708a78618d597dee15d0dc989f093ca5f9120 /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA
> -done
> +export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1/
> +# load prefetch cache
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo cee13b2ea505a7f37bd20d271c6bc7e5f8d2dfcb

OK, which particular file is this?

>  metrics=$(curl http://127.0.0.1:$PORT1/metrics)
> -regex="fdcache_op_count\{op=\"enqueue\"\} ([0-9]+).*fdcache_op_count\{op=\"evict\"\} ([0-9]+).*"
> -enqueue=0
> -if [[ $metrics =~ $regex ]]
> -then
> -   enqueue=${BASH_REMATCH[1]}
> -   evict=${BASH_REMATCH[2]}
> +regex="fdcache_prefetch_count ([0-9])+"
> +# Check to see if prefetch cache is loaded
> +if [[ $metrics =~ $regex ]]; then
> +  if [[  ${BASH_REMATCH[1]} -ne 1 ]]; then
> +    err
> +  fi

Why the -ne 1 ? What number are we expecting and why?

>  else
> -   err
> -fi
> -# This is an ad-hoc test than only works when FDCACHE_FDS < "archive_test"s above (5)
> -# otherwise there will be no eviction
> -if [[ $(( $enqueue-$FDCACHE_FDS )) -ne $evict ]]
> -then
> -   err
> -fi
> -# Test prefetch cache
> -archive_test bc1febfd03ca05e030f0d205f7659db29f8a4b30 /usr/src/debug/hello-1.0/hello.c $SHA
> +  err
> +fi 
> +# Ensure that searching for the source prefetched finds it in the prefetch cache
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find source cee13b2ea505a7f37bd20d271c6bc7e5f8d2dfcb /usr/src/debug/hello.c
>  metrics=$(curl http://127.0.0.1:$PORT1/metrics)
> -regex="fdcache_prefetch_count ([0-9])+"
> -pf_count=0
> -for i in $FDCACHE_FDS
> -do
> -  if [[ $metrics =~ $regex ]]; then
> -
> -    if [[ $(( $pf_count+$PREFETCH )) -ne ${BASH_REMATCH[1]} ]]; then
> -      err
> -    else
> -      pf_count=${BASH_REMATCH[1]}
> -    fi
> -
> -  else
> +regex="fdcache_op_count\{op=\"prefetch_access\"\} ([0-9])+"
> +if [[ $metrics =~ $regex ]]; then
> +  if [[  ${BASH_REMATCH[1]} -ne 1 ]]; then
>      err
>    fi
> -done
> +else
> +  err
> +fi

Again -ne 1? Shouldn't it be different from the above now?
 
>  kill $PID1
>  wait $PID1
>  PID1=0
>  
> -
>  #########
> -# Test mb limit on fd and prefetch cache
> +# Test mb limit on fd cache
>  #########
> -
>  rm -rf $DEBUGINFOD_CACHE_PATH
>  rm -rf $DB
>  env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod $VERBOSE -p $PORT1 -d $DB \
> -    --fdcache-mbs=1 --fdcache-prefetch-mbs=1 --fdcache-mintmp 0 -vvvvv -g 0 -t 0 -R R \
> -    > vlog2$PORT1 2>&1 &
> +    --fdcache-mbs=$FDCACHE_MBS --fdcache-mintmp 0 -vvvvv -g 0 -t 0 -R R > vlog2$PORT1 2>&1 &
>  PID1=$!

OK start a fresh debuginfod with -fdcache-mbs=1 --fdcache-mintmp 0
Is the fdcache-mintmp 0 meant to clear the cache immediately?

>  tempfiles vlog2$PORT1
>  errfiles vlog2$PORT1
> @@ -139,26 +119,26 @@ wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
>  # All rpms need to be in the index, except the dummy permission-000 one
>  rpms=$(find R -name \*rpm | grep -v nothing | wc -l)
>  wait_ready $PORT1 'scanned_files_total{source=".rpm archive"}' $rpms
> -kill -USR1 $PID1  # two hits of SIGUSR1 may be needed to resolve .debug->dwz->srefs
> -# Wait till both files are in the index and scan/index fully finished
> +kill -USR1 $PID1
>  wait_ready $PORT1 'thread_work_total{role="traverse"}' 2
> +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
> +wait_ready $PORT1 'thread_busy{role="scan"}' 0

OK, boilerplate again for triggering and waiting for a new scan to
complete.

> -archive_test c36708a78618d597dee15d0dc989f093ca5f9120 /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA
> +# This many archives cause the fd cache to be loaded with just over 1mb of
> +# files. So we expect the 1mb cap off
> +SHA=f4a1a8062be998ae93b8f1cd744a398c6de6dbb1
>  archive_test bc1febfd03ca05e030f0d205f7659db29f8a4b30 /usr/src/debug/hello-1.0/hello.c $SHA
>  archive_test c36708a78618d597dee15d0dc989f093ca5f9120 /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA
>  archive_test 41a236eb667c362a1c4196018cc4581e09722b1b /usr/src/debug/hello2-1.0-2.x86_64/hello.c $SHA
>  archive_test bc1febfd03ca05e030f0d205f7659db29f8a4b30 /usr/src/debug/hello-1.0/hello.c $SHA
>  archive_test f0aa15b8aba4f3c28cac3c2a73801fefa644a9f2 /usr/src/debug/hello-1.0/hello.c $SHA
>  archive_test bbbf92ebee5228310e398609c23c2d7d53f6e2f9 /usr/src/debug/hello-1.0/hello.c $SHA
> -archive_test d44d42cbd7d915bc938c81333a21e355a6022fb7 /usr/src/debug/hello-1.0/hello.c $SHA

OK, test that the hello.c source file is found in the scanned archives,
that the fdcache is used and that the sha1sum checks out.

>  metrics=$(curl http://127.0.0.1:$PORT1/metrics)
> -regex="fdcache_bytes ([0-9]+).*fdcache_prefetch_bytes ([0-9]+)"
> +regex="fdcache_bytes ([0-9]+)"
>  mb=$((1024*1024))
>  if [[ $metrics =~ $regex ]]; then
>    fdbytes=${BASH_REMATCH[1]}
> -  pfbytes=${BASH_REMATCH[2]}
> -  if [ $fdbytes -gt $mb ] || [ $pfbytes -gt $mb ]; then
> +  if [ $fdbytes -gt $mb ] ; then
>      err
>    fi

OK, check that the prefetch cache isn't bigger than 10MB. Why that
number?

Thanks,

Mark


  reply	other threads:[~2022-05-27 16:20 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-25 18:44 Noah Sanci
2022-05-27 16:20 ` Mark Wielaard [this message]
     [not found]   ` <CAJXA7qiYfeEpr5pHY5xPWcmnMoA1csnBvDnAHBoTubTVyBz+4g@mail.gmail.com>
2022-05-30 15:36     ` Mark Wielaard
2022-06-03 14:49       ` Mark Wielaard
     [not found]         ` <CAJXA7qjnhkwRaMWv2C7xnG+ruWFVsPgrvufDG94NUQe2Z067nA@mail.gmail.com>
2022-06-03 15:02           ` Mark Wielaard
2022-06-03 16:22             ` Mark Wielaard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ede645fbb9d97c1e7d2c97605e8bd684074289ab.camel@klomp.org \
    --to=mark@klomp.org \
    --cc=elfutils-devel@sourceware.org \
    --cc=nsanci@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).