Re: Flushing large data immediately in pqcomm

On 14/03/2024 13:22, Melih Mutlu wrote:
> @@ -1282,14 +1283,32 @@ internal_putbytes(const char *s, size_t len)
> if (internal_flush())
> return EOF;
> }
> - amount = PqSendBufferSize - PqSendPointer;
> - if (amount > len)
> - amount = len;
> - memcpy(PqSendBuffer + PqSendPointer, s, amount);
> - PqSendPointer += amount;
> - s += amount;
> - len -= amount;
> +
> + /*
> + * If the buffer is empty and data length is larger than the buffer
> + * size, send it without buffering. Otherwise, put as much data as
> + * possible into the buffer.
> + */
> + if (!pq_is_send_pending() && len >= PqSendBufferSize)
> + {
> + int start = 0;
> +
> + socket_set_nonblocking(false);
> + if (internal_flush_buffer(s, &start, (int *)&len))
> + return EOF;
> + }
> + else
> + {
> + amount = PqSendBufferSize - PqSendPointer;
> + if (amount > len)
> + amount = len;
> + memcpy(PqSendBuffer + PqSendPointer, s, amount);
> + PqSendPointer += amount;
> + s += amount;
> + len -= amount;
> + }
> }
> +
> return 0;
> }

Two small bugs:

- the "(int *) &len)" cast is not ok, and will break visibly on
big-endian systems where sizeof(int) != sizeof(size_t).

- If internal_flush_buffer() cannot write all the data in one call, it
updates 'start' for how much it wrote, and leaves 'end' unchanged. You
throw the updated 'start' value away, and will send the same data again
on next iteration.

Not a correctness issue, but instead of pq_is_send_pending(), I think it
would be better to check "PqSendStart == PqSendPointer" directly, or
call socket_is_send_pending() directly here. pq_is_send_pending() does
the same, but it's at a higher level of abstraction.

--
Heikki Linnakangas
Neon (https://neon.tech)