Em seg., 22 de jun. de 2020 às 16:33, Robert Haas <robertmh...@gmail.com> escreveu:
> Ranier, > > This topic is largely unrelated to the current thread. Also... > Weel, I was trying to improve the patch for the current thread. Or perhaps, you are referring to something else, which I may not have understood. > > On Mon, Jun 22, 2020 at 12:47 PM Ranier Vilela <ranier...@gmail.com> > wrote: > > Questions: > > 1. Why acquire and release lock in retry: loop. > > This is a super-bad idea. Note the coding rule mentioned in spin.h. > There are many discussion on this mailing list about the importance of > keeping the critical section for a spinlock to a few instructions. > Calling another function that *itself acquires an LWLock* is > definitely not OK. > Perhaps, I was not clear and it is another misunderstanding. I am not suggesting a function to acquire the lock. By the way, I did the tests with this change and it worked perfectly. But, as it is someone else's patch, I asked why to learn. By the way, my suggestion is with less instructions than the patch. The only change I asked is why to acquire and release the lock repeatedly within the goto retry, when you already have it. If I can acquire the lock before retry: and release it only at the end when I leave table_block_parallelscan_startblock_init, why not do it. I will attach the suggested excerpt so that I have no doubts about what I am asking. regards, Ranier Vilela
void table_block_parallelscan_startblock_init(Relation rel, ParallelBlockTableScanWork workspace, ParallelBlockTableScanDesc pbscan) { BlockNumber sync_startpage = InvalidBlockNumber; /* Reset the state we use for controlling allocation size. */ memset(workspace, 0, sizeof(*workspace)); StaticAssertStmt(MaxBlockNumber <= 0xFFFFFFFE, "pg_nextpower2_32 may be too small for non-standard BlockNumber width"); workspace->phsw_chunk_size = pg_nextpower2_32(Max(pbscan->phs_nblocks / PARALLEL_SEQSCAN_NCHUNKS, 1)); /* Ensure we don't go over the maximum size with larger rels */ workspace->phsw_chunk_size = Min(workspace->phsw_chunk_size, PARALLEL_SEQSCAN_MAX_CHUNK_SIZE); /* Grab the spinlock. */ SpinLockAcquire(&pbscan->phs_mutex); retry: /* * If the scan's startblock has not yet been initialized, we must do so * now. If this is not a synchronized scan, we just start at block 0, but * if it is a synchronized scan, we must get the starting position from * the synchronized scan machinery. We can't hold the spinlock while * doing that, though, so release the spinlock, get the information we * need, and retry. If nobody else has initialized the scan in the * meantime, we'll fill in the value we fetched on the second time * through. */ if (pbscan->phs_startblock == InvalidBlockNumber) { if (!pbscan->base.phs_syncscan) pbscan->phs_startblock = 0; else if (sync_startpage != InvalidBlockNumber) pbscan->phs_startblock = sync_startpage; else { sync_startpage = ss_get_location(rel, pbscan->phs_nblocks); goto retry; } } SpinLockRelease(&pbscan->phs_mutex); }