RE: Reconnect a single connection used by multiple threads in embedded SQL in C application causes error.

From: "egashira(dot)yusuke(at)fujitsu(dot)com" <egashira(dot)yusuke(at)fujitsu(dot)com>
To: 'Noah Misch' <noah(at)leadboat(dot)com>
Cc: 'Kyotaro Horiguchi' <horikyota(dot)ntt(at)gmail(dot)com>, "pgsql-bugs(at)lists(dot)postgresql(dot)org" <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: RE: Reconnect a single connection used by multiple threads in embedded SQL in C application causes error.
Date: 2022-03-25 13:34:16
Message-ID: TYWPR01MB720228CE3707954B3EDA67A3FF1A9@TYWPR01MB7202.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

Hi.

Thank you for the comment.
I attached the revised patch v3.

> On Fri, Mar 04, 2022 at 11:49:39AM +0000, egashira(dot)yusuke(at)fujitsu(dot)com wrote:
> > > It looks like too verbose. Couldn't it be summarized as something
> > > like this?
> > >
> > > > Current connection is managed in a per-thread manner. For the first
> > > > use on a thread, it is the most recently used one in the process.
>
> There's no behavior specific to "the first use on a thread", and "most
> recently used" is not a factor. Avoid those phrases. (In a thread that has
> never opened a connection, each command that retrieves the current connection
> will retrieve the non-thread-specific current connection.)
>
> > Yes.
> > I thought that I needed a detailed explanation of the current connection for the note below, but this certainly seems to be summable.
>
> For this particular paragraph, I liked your v1's level of verbosity more than
> this compact version.

I have reinstated v1's explanation of this paragraph because the current connection is relevant to the subsequent note.

> > I wanted to make a document that could avoid the use cases that I reported, so I tried to write a note according to the occurrence conditions.
> > However, this was also too verbose, so I rewrote the note as you suggested.
> > And since there is no mention of the current connection, I changed the section to add it to "36.2.3. Closing a Connection".
> >
> > I think I might be better to add "In the worst case, this may cause an application crash." to this note so that users can avoid the problem more strongly.
> > However, this sentence can also be read as claiming that there is an implementation problem.
> > Should not I add this sentence?
>
> It's okay to admit implementation problems. I might write "Behavior is
> undefined if the current thread is not the thread that opened the connection."
> Referring to undefined behavior is a customary way for API docs to imply that
> a crash is possible. If one wanted to be even more precise, "Behavior is
> undefined if the connection is the thread-specific current connection of any
> other thread." However, the more-precise wording is harder to understand.
> Also, being more precise now could limit our freedom to change behavior later.

The most important thing I want to explain in the documentation is that the current connection should not be closed from another thread.
So I just added that to the DISCONNECT Note.

> > --- a/doc/src/sgml/ecpg.sgml
> > +++ b/doc/src/sgml/ecpg.sgml
> > @@ -222,10 +222,15 @@ EXEC SQL CONNECT TO <replaceable>target</replaceable> <optional>AS <replaceable>
> > <para>
> > The <replaceable>connection-name</replaceable> is used to handle
> > multiple connections in one program. It can be omitted if a
> > - program uses only one connection. The most recently opened
> > - connection becomes the current connection, which is used by default
> > - when an SQL statement is to be executed (see later in this
> > - chapter).
> > + program uses only one connection.
> > + </para>
> > +
> > + <para>
> > + The most recently opened connection becomes the current connection.
>
> This is true. Disconnecting also changes the current connection, to another
> one of the open connections. Please document that somewhere appropriate.

I have added that behavior to the "Closing a Connection" section.

> > @@ -276,8 +281,8 @@ EXEC SQL CONNECT TO :target USER :user USING :passwd;
> >
> > <para>
> > SQL statements in embedded SQL programs are by default executed on
> > - the current connection, that is, the most recently opened one. If
> > - an application needs to manage multiple connections, then there are
> > + the current connection, that is, the most recently opened one in each thread.
>
> Likewise, this is not accurate if a disconnect of the current connection
> happened after the most recent open. You could simply delete this claim,
> which would avoid defining the current connection in two places.

As I explained the current connection in detail before this section, I think that there is no need to supplement the definition of the current connection.
Therefore, I removed the second half of this statement.

Regards,
Yusuke Egashira

Attachment Content-Type Size
v3-add-multithreaded-note-to-ecpg-connection-document.patch application/octet-stream 2.4 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message PG Bug reporting form 2022-03-25 15:01:44 BUG #17449: Disk space not released
Previous Message hubert depesz lubaczewski 2022-03-25 08:38:56 Re: Logical replication stops dropping used initial-sync replication slots