Poor Blob performance over high latency connection

[wegendt-bosch]

Describe the issue
The PreparedStatement class has poor performance for storing Blobs as oids over high latency network connections, since it uses a fixed buffer size of 4 KB.

Driver Version?
latest

Java Version?
any

OS Version?
Win 10

PostgreSQL Version?
latest

To Reproduce
Steps to reproduce the behaviour:
Create a Blob and store it as oid with a network with high latency (e.g. over internet)

Expected behaviour
A clear and concise description of what you expected to happen.
And what actually happens
Transfer rates approaching network speed

Possible fix:
make PgPreparedStatement::createBlob use a configurable buffer size via e.g. org/postgresql/driverconfig.properties. Suggested name: binaryTransferBufferSize

[davecramer]

Care to create a PR ?

[wegendt-bosch]

I need to go through some hoops since I'm in some corp, so it will take some time, but if you haven't done it till then I will definitely!

[vlsi]

@wegendt-bosch could you please clarify how you use createBlob?
Do you pass limit parameter? What is the type of InputStream you pass?
I guess we could use Math.min(64*1024, length) for the buffer size, and it might be enough.

I am not fond of adding yet another configuration source, so if we consider making blob buffer size configurable, we should probably use connection properties for that.

What is the source of the InputStream? Is it FileInputStream or something else?

---

Yet another direction for reducing the latency would be pipelining the requests. As createBlob is a single JDBS call, it could pipeline lowrite requests, and there's no need for the driver to wait for the response after each lowrite request. The driver could send a bunch of data, and eventually monitor the progress.

[vlsi]

One more question: do you have any benchmarks for the buffer size in your environment?
What is the sweet spot, and what if we just use your value rather than hard-coded 4096?

[wegendt-bosch]

Hi,

I don't call the method myself, I use hibernate and that calls it. Therefore I can't use connection properties, since hibernate does not forward those AFAIK.

Using a buffer size of 128KB is about 10 times faster, but I'm not sure you want to reserve that much RAM for transfers in local networks, this really is just for high-ping environments.

The source of the input stream is a hibernate-internal stream. If you want to pursue a different direction you should probably do it instead of me, I'm just showing you my solution I'm using in a fork until this bug is resolved :)

[wegendt-bosch]

Math.min(64*1024, length)

You probably mean Math.max, right?

[vlsi]

You probably mean Math.max, right?

I mean we should:

1. Use blob.length if the blob is small. For instance, if the blob is 100 bytes, we should not create 256KiB buffer
2. Use a limited a mount of memory. For instance, if the blobl is 100 megabytes, we should probably use a fixed buffer of 64 or 256 KiB otherwise we might run into OutOfMemoryError

That is why I suggested min

---

The source of the input stream is a hibernate-internal stream. If you want to pursue a different direction you should probably do it instead of me, I'm just showing you my solution I'm using in a fork until this bug is resolved :)

A slightly different idea might be storing the InputStream as is, and then read that stream somewhere like in

pgjdbc/pgjdbc/src/main/java/org/postgresql/util/StreamWrapper.java

Lines 124 to 134 in 65f158e

	private static int copyStream(InputStream inputStream, OutputStream outputStream, int limit)
	throws IOException {
	int totalLength = 0;
	byte[] buffer = new byte[2048];
	int readLength = inputStream.read(buffer);
	while (readLength > 0) {
	totalLength += readLength;
	outputStream.write(buffer, 0, readLength);
	if (totalLength >= limit) {
	return -1;
	}

Do you know the exact "hibernate-internal stream" or can you suggest a reproducer?
If "hibeernate-internal stream" is something like ByteArrayInputStream, then we could probably borrow the byte array from it (with contents!) rather than read it bite-by-bite.

[wegendt-bosch]

In hibernate's case, it is a org.hibernate.engine.jdbc.internal.BinaryStreamImpl extends ByteArrayInputStream, but since you are maintaining a library I'd assume you'd want to solve the general case, too, so at least have performant handling for arbitrary streams, and as an extension optimize specific cases, too (which would then be an extension of my proposed changes).

I don't see what's the difference of the copyStream method and the one in createBlob, the blocking/non-performant call is the one to outputStream.write with small buffers, no?

I'll add the Math.min to allocating the buffer to handle small blobs :)

[wegendt-bosch]

to reproduce with hibernate, create an entity holding a blob:

and assign a blob using some hibernate blob creator, e.g.:

blobHolder.theBlob = NonContextualLobCreator.INSTANCE.createBlob(inputStream.readAllBytes())

then store it in postgres:

session.save(blobHolder)

[vlsi]

org.hibernate.engine.jdbc.internal.BinaryStreamImpl extends ByteArrayInputStream, but since you are maintaining a library I'd assume you'd want to solve the general case

Right you are. However, we can still consider adding optimization like if (inputStream extends ByteArrayInputStream) { skip buffering, and just reuse the stream as if it was already buffered }. I think it would keep the semantics, and it would avoid excessive copying of the data between buffers. It might be quite common for the users to pass ByteArrayInputStream instances, so the optimization might be helpful for generic case, not just Hibernate.

[wegendt-bosch]

Hm, it seems that ByteArrayInputStream does not expose its array, I'd guess this is since it also is a sliced view of the array, i.e. it has members offset, pos and count.

[vlsi]

Hm, it seems that ByteArrayInputStream does not expose its array

It exposes java.io.ByteArrayInputStream#transferTo(OutputStream out).

[wegendt-bosch]

ah sure, I can use that

[wegendt-bosch]

Compiler says, transferTo is available since Java9, I think you want to keep Java8 compatibility...?

[vlsi]

Compiler says, transferTo is available since Java9, I think you want to keep Java8 compatibility

We would like to keep Java 8 compatibility indeed.
However, we can probably go for reflection for "pre Java 9" and interface call for Java 9+.

[vlsi]

ByteArrayInputStream.buf field is protected, so we don't even need to use reflection to get its value. We can craft ByteBufferInputStreamBufferExposer extends ByteArrayInputStream which could expose transferTo.

[wegendt-bosch]

I don't think you can extend existing objects, I think for Java8 reflection is the only way you could achieve this

[wegendt-bosch]

Fixed by #3044