Re: Assert("vacrel->eager_scan_remaining_successes > 0")

On Thu, May 22, 2025 at 7:27 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> On Wed, May 21, 2025 at 6:11 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > > if (vacrel->eager_scan_remaining_successes > 0)
> > >      vacrel->eager_scan_remaining_successes--;
> >
> > I've attached a patch that uses this idea. Feedback is very welcome.
>
> Thanks for writing the patch!
>
> I actually think we have the same situation with
> eager_scan_remaining_fails.

Good catch.

> Since the extra pages that are eagerly
> scanned could either fail or succeed to be frozen, so we probably also
> need to change the assert in the failure case into a guard as well:
>
>             else
>             {
>                 Assert(vacrel->eager_scan_remaining_fails > 0);
>                 vacrel->eager_scan_remaining_fails--;
>             }
>
> ->
>
>           else if (vacrel->eager_scan_remaining_fails > 0)
>                vacrel->eager_scan_remaining_fails--;
>
> In the comment you wrote, I would probably just change one thing
>
> +                    /*
> +                     * Report only once that we disabled eager scanning. This
> +                     * check is required because we might have eagerly read
> +                     * more blocks and we could reach here even after
> +                     * disabling eager scanning.
> +                     */
>
> I would emphasize that we read ahead these blocks before executing the
> code trying to freeze them. So, I might instead say something like:
> "Report only once that we disabled eager scanning. We may eagerly read
> ahead blocks in excess of the success or failure caps before
> attempting to freeze them, so we could reach here even after disabling
> additional eager scanning"
>
> And then probably avoid repeating the whole comment above the
> remaining fails guard.

Agreed. I've updated the patch. Does this address your comments?

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com