Обсуждение: Misleading comment in prologue of ReorderBufferQueueMessage
The name of the function suggests that the given message will be queued in ReorderBuffer. The prologue of the function says so too
776 /*
777 * Queue message into a transaction so it can be processed upon commit.
778 */
777 * Queue message into a transaction so it can be processed upon commit.
778 */
It led me to think that a non-transactional message is processed along with the surrounding transaction, esp. when it has an associated xid.
But in reality, the function queues only a transactional message and decoders a non-transactional message immediately without waiting for a commit.
We should modify the prologue to say
"Queue a transactional message into a transaction so that it can be processed upon commit. A non-transactional message is processed immediately." and also change the name of the function to ReorderBufferProcessMessage(), but the later may break API compatibility.
--
Best Wishes,
Ashutosh
On Mon, Dec 14, 2020 at 2:45 PM Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: > > The name of the function suggests that the given message will be queued in ReorderBuffer. The prologue of the functionsays so too > 776 /* > 777 * Queue message into a transaction so it can be processed upon commit. > 778 */ > It led me to think that a non-transactional message is processed along with the surrounding transaction, esp. when it hasan associated xid. > > But in reality, the function queues only a transactional message and decoders a non-transactional message immediately withoutwaiting for a commit. > > We should modify the prologue to say > "Queue a transactional message into a transaction so that it can be processed upon commit. A non-transactional messageis processed immediately." and also change the name of the function to ReorderBufferProcessMessage(), but the latermay break API compatibility. > +1 for the comment change but I am not sure if it is a good idea to change the API name. -- With Regards, Amit Kapila.
On Mon, Dec 14, 2020 at 3:14 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, Dec 14, 2020 at 2:45 PM Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
>
> The name of the function suggests that the given message will be queued in ReorderBuffer. The prologue of the function says so too
> 776 /*
> 777 * Queue message into a transaction so it can be processed upon commit.
> 778 */
> It led me to think that a non-transactional message is processed along with the surrounding transaction, esp. when it has an associated xid.
>
> But in reality, the function queues only a transactional message and decoders a non-transactional message immediately without waiting for a commit.
>
> We should modify the prologue to say
> "Queue a transactional message into a transaction so that it can be processed upon commit. A non-transactional message is processed immediately." and also change the name of the function to ReorderBufferProcessMessage(), but the later may break API compatibility.
>
+1 for the comment change but I am not sure if it is a good idea to
change the API name.
Can you please review wording? I will create a patch with updated wording.
--
Best Wishes,
Ashutosh
On Tue, Dec 15, 2020 at 11:25 AM Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: > > On Mon, Dec 14, 2020 at 3:14 PM Amit Kapila <amit.kapila16@gmail.com> wrote: >> >> On Mon, Dec 14, 2020 at 2:45 PM Ashutosh Bapat >> <ashutosh.bapat@enterprisedb.com> wrote: >> > >> > The name of the function suggests that the given message will be queued in ReorderBuffer. The prologue of the functionsays so too >> > 776 /* >> > 777 * Queue message into a transaction so it can be processed upon commit. >> > 778 */ >> > It led me to think that a non-transactional message is processed along with the surrounding transaction, esp. when ithas an associated xid. >> > >> > But in reality, the function queues only a transactional message and decoders a non-transactional message immediatelywithout waiting for a commit. >> > >> > We should modify the prologue to say >> > "Queue a transactional message into a transaction so that it can be processed upon commit. A non-transactional messageis processed immediately." and also change the name of the function to ReorderBufferProcessMessage(), but the latermay break API compatibility. >> > >> >> +1 for the comment change but I am not sure if it is a good idea to >> change the API name. >> > Can you please review wording? I will create a patch with updated wording. > How about something like below: A transactional message is queued to be processed upon commit and a non-transactional message gets processed immediately. OR A transactional message is queued so it can be processed upon commit and a non-transactional message gets processed immediately. -- With Regards, Amit Kapila.
On Wed, Dec 16, 2020 at 8:00 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Tue, Dec 15, 2020 at 11:25 AM Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
>
> On Mon, Dec 14, 2020 at 3:14 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Mon, Dec 14, 2020 at 2:45 PM Ashutosh Bapat
>> <ashutosh.bapat@enterprisedb.com> wrote:
>> >
>> > The name of the function suggests that the given message will be queued in ReorderBuffer. The prologue of the function says so too
>> > 776 /*
>> > 777 * Queue message into a transaction so it can be processed upon commit.
>> > 778 */
>> > It led me to think that a non-transactional message is processed along with the surrounding transaction, esp. when it has an associated xid.
>> >
>> > But in reality, the function queues only a transactional message and decoders a non-transactional message immediately without waiting for a commit.
>> >
>> > We should modify the prologue to say
>> > "Queue a transactional message into a transaction so that it can be processed upon commit. A non-transactional message is processed immediately." and also change the name of the function to ReorderBufferProcessMessage(), but the later may break API compatibility.
>> >
>>
>> +1 for the comment change but I am not sure if it is a good idea to
>> change the API name.
>>
> Can you please review wording? I will create a patch with updated wording.
>
How about something like below:
A transactional message is queued to be processed upon commit and a
non-transactional message gets processed immediately.
Used this one. PFA patch.
--
Best Wishes,
Ashutosh
Вложения
On Fri, Dec 18, 2020 at 3:37 PM Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote: > > On Wed, Dec 16, 2020 at 8:00 AM Amit Kapila <amit.kapila16@gmail.com> wrote: >> >> How about something like below: >> A transactional message is queued to be processed upon commit and a >> non-transactional message gets processed immediately. > > > Used this one. PFA patch. > Pushed! -- With Regards, Amit Kapila.