Re: [PATCH] Slight improvement of worker_spi.c example

> On 10 Jul 2023, at 14:40, Aleksander Alekseev > wrote: >> Have you had a chance to look at these suggestions, and Juliens reply >> downthread, in order to produce a new version of the patch? > > Thanks for the reminder. No I haven't. Please feel free marking this > CF entry as RwF if this wil

Re: [PATCH] Slight improvement of worker_spi.c example

Hi, > Have you had a chance to look at these suggestions, and Juliens reply > downthread, in order to produce a new version of the patch? Thanks for the reminder. No I haven't. Please feel free marking this CF entry as RwF if this will be helpful. We may reopen it if and when necessary. -- Best

Re: [PATCH] Slight improvement of worker_spi.c example

> On 14 Jun 2023, at 13:08, Aleksander Alekseev > wrote: >> Are you or Aleksander interested in helping improve this module? I'm happy >> to help review and/or write patches. > > Unfortunately I'm not familiar with the problem in respect of naptime > Julien is referring to. If you know what th

Re: [PATCH] Slight improvement of worker_spi.c example

On Wed, Jun 14, 2023 at 02:08:03PM +0300, Aleksander Alekseev wrote: > > Unfortunately I'm not familiar with the problem in respect of naptime > Julien is referring to. If you know what this problem is and how to > fix it, go for it. I'll review and test the code then. I can write the > part of the

Re: [PATCH] Slight improvement of worker_spi.c example

Hi Nathan, > > That being said, I still don't understand why you focus on this tiny and not > > really important detail while the module itself is actually broken (for > > dynamic > > bgworker without s_p_l) and also has some broken behaviors with regards to > > the > > naptime that are way more

Re: [PATCH] Slight improvement of worker_spi.c example

On Tue, Jun 13, 2023 at 07:58:02PM +0800, Julien Rouhaud wrote: > That being said, I still don't understand why you focus on this tiny and not > really important detail while the module itself is actually broken (for > dynamic > bgworker without s_p_l) and also has some broken behaviors with regar

Re: [PATCH] Slight improvement of worker_spi.c example

On Tue, Jun 13, 2023 at 12:34:09PM +0300, Aleksander Alekseev wrote: > > > I agree that the current code > > could lead folks to think that PushActiveSnapshot must go after > > SPI_connect, but wouldn't the reverse ordering just give folks the opposite > > impression? > > This is the exact reason w

Re: [PATCH] Slight improvement of worker_spi.c example

Hi, > On Sat, Jun 03, 2023 at 06:35:00PM -0400, Michael Paquier wrote: > > It does not have to be complicated, but I definitely agree that we'd > > better spend some efforts in improving it as a whole especially > > knowing that this is mentioned on the docs as an example that one > > could rely o

Re: [PATCH] Slight improvement of worker_spi.c example

On Sat, Jun 03, 2023 at 06:35:00PM -0400, Michael Paquier wrote: > It does not have to be complicated, but I definitely agree that we'd > better spend some efforts in improving it as a whole especially > knowing that this is mentioned on the docs as an example that one > could rely on. +1. I know

Re: [PATCH] Slight improvement of worker_spi.c example

Hi Michael, Thanks for your feedback. > + * The order of PushActiveSnapshot() and SPI_connect() is not really > + * important. > > FWIW, looking at the patch, I don't think that this is particularly > useful. Fair enough, here is the corrected patch. -- Best regards, Aleksander Ale

Re: [PATCH] Slight improvement of worker_spi.c example

On Sat, Jun 03, 2023 at 03:34:30PM +0300, Aleksander Alekseev wrote: > Agree. It is a simple example and I don't think it's going to be > useful to make a complicated one out of it. It does not have to be complicated, but I definitely agree that we'd better spend some efforts in improving it as a

Re: [PATCH] Slight improvement of worker_spi.c example

Hi, > That being said this module is really naive and has so many problems that I > don't think it's actually helpful as coding guidelines for anyone who wants to > create a non toy extension using bgworkers. Agree. It is a simple example and I don't think it's going to be useful to make a compli

Re: [PATCH] Slight improvement of worker_spi.c example

On Sat, Jun 03, 2023 at 02:38:26PM +0300, Aleksander Alekseev wrote: > Hi Julien, > > > I'm pretty sure that this is intentional. The worker can be launched > > dynamically and in that case it still needs a GUC for the naptime. > > The dynamic worker also is going to need worker_spi_database, howe

Re: [PATCH] Slight improvement of worker_spi.c example

Hi Julien, > I'm pretty sure that this is intentional. The worker can be launched > dynamically and in that case it still needs a GUC for the naptime. The dynamic worker also is going to need worker_spi_database, however the corresponding GUC declaration is placed below the check. Perhaps we sh

Re: [PATCH] Slight improvement of worker_spi.c example

On Sat, Jun 03, 2023 at 02:09:26PM +0300, Aleksander Alekseev wrote: > > Additionally I noticed that the check: > > ``` > if (!process_shared_preload_libraries_in_progress) > return; > ``` > > ... was misplaced in _PG_init(). Here is the patch v2 which fixes this too. I'm p

Re: [PATCH] Slight improvement of worker_spi.c example

Hi, > The patch changes the order to: > > StartTransactionCommand(); > PushActiveSnapshot(...); > SPI_connect(); > > ... > > SPI_finish(); > PopActiveSnapshot(); > CommitTransactionCommand(); > > ... and also clarifies that the order of PushActiveSnapshot(...) and > SPI