On 2021-08-18 1:29 a.m., Kyotaro Horiguchi wrote:
At Tue, 17 Aug 2021 17:04:44 +0900, Michael Paquier <mich...@paquier.xyz> wrote 
in
On Fri, Jul 02, 2021 at 06:22:56PM -0300, Ranier Vilela wrote:
Em qui., 1 de jul. de 2021 às 17:20, Mahendra Singh Thalor <
mahi6...@gmail.com> escreveu:
Please can we try to hit this rare condition by any test case. If you have
any test cases, please share.
Yeah, this needs to be proved.  Are you sure that this change is
actually right?  The bottom of FreePageManagerPutInternal() has
assumptions that a page may not be found during a btree search, with
an index value used.
By a quick look, FreePageBtreeSearch is called only from
FreePageManagerPutInternal at three points. The first one assumes that
result.found == true, at the rest points are passed only when
fpm->btree_depth > 0, i.e, fpm->btree_root is non-NULL.

In short FreePageBtreeSeach is never called when fpm->btree_root is
NULL.  I don't think we need to fill-in other members since the
contract of the function looks fine.

It might be simpler to turn 'if (btp == NULL)' to an assertion.
After added the initialization of split_pages in patch fix_unitialized_var_index_freepage-v1.patch,

+        result->split_pages = 0;

it actually changed the assertion condition after the second time function call of FreePageBtreeSearch.
            FreePageBtreeSearch(fpm, first_page, &result);

            /*
             * The act of allocating pages for use in constructing our btree
             * should never cause any page to become more full, so the new
             * split depth should be no greater than the old one, and perhaps              * less if we fortuitously allocated a chunk that freed up a slot
             * on the page we need to update.
             */
            Assert(result.split_pages <= fpm->btree_recycle_count);

Should we consider adding some test cases to make sure this assertion will still function properly?


regards.

--
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca


Reply via email to