Introduce converters to consume Postgres Json and convert these to String/byte[]

[jdmwood]

I think I have found a potential memory leak using Spring Data + R2BC Posgres.

I have a model and repository like this (Kotlin code):

@Table
data class MyTable(
    val data: Json
)

interface MyTableRepository : CoroutineCrudRepository<MyTable, Int> {
}

The SQL to create this table is:

create table my_table (
     data json not null
);

If I do a query like this:

       for (i in 0..1000) {
            testRepo.findAll().toList()
        }

... then I get a bunch of Netty memory leak errors (https://netty.io/wiki/reference-counted-objects.html). The culprit seems to be this one:

2020-09-09 13:26:51.761 ERROR 78212 --- [actor-tcp-nio-2] io.netty.util.ResourceLeakDetector       : LEAK: ByteBuf.release() was not called before it's garbage-collected. See https://netty.io/wiki/reference-counted-objects.html for more information.
Recent access records: 
Created at:
	io.netty.buffer.SimpleLeakAwareByteBuf.unwrappedDerived(SimpleLeakAwareByteBuf.java:143)
	io.netty.buffer.SimpleLeakAwareByteBuf.retainedSlice(SimpleLeakAwareByteBuf.java:52)
	io.netty.buffer.AdvancedLeakAwareByteBuf.retainedSlice(AdvancedLeakAwareByteBuf.java:89)
	io.r2dbc.postgresql.codec.JsonCodec.doDecode(JsonCodec.java:49)
	io.r2dbc.postgresql.codec.JsonCodec.doDecode(JsonCodec.java:34)
	io.r2dbc.postgresql.codec.AbstractCodec.decode(AbstractCodec.java:82)
	io.r2dbc.postgresql.codec.DefaultCodecs.decode(DefaultCodecs.java:129)
	io.r2dbc.postgresql.PostgresqlRow.decode(PostgresqlRow.java:90)
	io.r2dbc.postgresql.PostgresqlRow.get(PostgresqlRow.java:77)
	io.r2dbc.spi.Row.get(Row.java:78)
	org.springframework.data.r2dbc.convert.MappingR2dbcConverter$RowParameterValueProvider.getParameterValue(MappingR2dbcConverter.java:656)
	org.springframework.data.r2dbc.convert.MappingR2dbcConverter$$Lambda$1333/0000000000000000.apply(Unknown Source)
	org.springframework.data.relational.core.conversion.BasicRelationalConverter$ConvertingParameterValueProvider.getParameterValue(BasicRelationalConverter.java:264)
	org.springframework.data.mapping.model.ClassGeneratingEntityInstantiator$EntityInstantiatorAdapter.extractInvocationArguments(ClassGeneratingEntityInstantiator.java:262)
	org.springframework.data.mapping.model.ClassGeneratingEntityInstantiator$EntityInstantiatorAdapter.createInstance(ClassGeneratingEntityInstantiator.java:235)
	org.springframework.data.mapping.model.ClassGeneratingEntityInstantiator.createInstance(ClassGeneratingEntityInstantiator.java:87)
	org.springframework.data.relational.core.conversion.BasicRelationalConverter.createInstance(BasicRelationalConverter.java:147)
	org.springframework.data.r2dbc.convert.MappingR2dbcConverter.createInstance(MappingR2dbcConverter.java:300)
	org.springframework.data.r2dbc.convert.MappingR2dbcConverter.read(MappingR2dbcConverter.java:121)
	org.springframework.data.r2dbc.convert.MappingR2dbcConverter.read(MappingR2dbcConverter.java:116)
	org.springframework.data.r2dbc.convert.EntityRowMapper.apply(EntityRowMapper.java:46)
...

Apologies if this is actually an issue in the core R2BC library. I'm happy to re-raise this there if that is the case.

Thanks!

[jdmwood]

OK digging into this a bit more, it appears that the issue is that the Json type is Json.JsonOutput which uses ByteBuf internally. It appears that it only cleans this up if you actually try and read the object.

Slightly weird behaviour I guess because if you don't read the value then it will leak. Maybe this is intentional?

[jdmwood]

One thing I should point out is that even if I change my model to:

@Table
data class MyTable(
    val data: String
)

... the problem still occurs, presumably because the R2DBC drivers are still doing the conversion to Json.

[mp911de]

Thanks for bringing up that issue.

As per Javadoc of Json:

JSON values should be generally considered for single-consumption only. Output values retain a reference to a potentially pooled memory buffer and must be consumed to avoid memory leaks.

Using Json in a mapped entity having a reference to a ByteBuf is error-prone and can quickly lead to memory issues as you've reported. The JSON object you're getting hold of uses a memory-efficient implementation. I assume that this arrangement is pretty common and it's not obvious where it comes from.

Mapping Json to string will likely render something like JsonByteBufInput{ {your JSON here} } into the field.

I see two things to do of which we can address one directly:

1. Introduce a Json to byte[]/String converter to consume Json objects that are backed by a ByteBuf. That's something we can do in Spring Data
2. Introduce an option in R2DBC Postgres to de-optimize Json consumption so that the driver copies the data from ByteBuf into a byte[] so that memory leaks cannot happen. Likely, that should be the default so that attached buffers become opt-in.

[jdmwood]

As a temporary workaround (in case this helps others), I made a new type with custom converters which always read the data exactly once into a GC collected string:

data class JsonString(
    val json: String
)

class JsonToJsonStringConverter : Converter<Json, JsonString> {
    override fun convert(from: Json): JsonString {
        return JsonString(from.asString())
    }
}

class JsonStringToJsonConverter : Converter<JsonString, Json> {
    override fun convert(from: JsonString): Json {
        return Json.of(from.json)
    }
}

@Bean
override fun r2dbcCustomConversions(): R2dbcCustomConversions {
    val converterList: MutableList<Converter<*, *>?> = ArrayList<Converter<*, *>?>()
    converterList.add(JsonToJsonStringConverter())
    converterList.add(JsonStringToJsonConverter())
    return R2dbcCustomConversions(storeConversions, converterList)
}

@Table
data class MyTable(
    val data: JsonString
)

[jdmwood]

Thanks for your reply @mp911de.

For the record, the super confusing thing is that even if I use a String in my mapped entity, I still get the leak. So simply not using Json isn't enough - if my native table uses json or jsonb datatypes this problem occurs, which is much less obvious for users.

[mp911de]

Can you file a ticket in https://github.com/pgjdbc/r2dbc-postgresql asking for an optimization switch for JSON? Probably the same would make sense for Clob and Blob in a way that the driver returns either detached objects (should be the default) or attached/memory-optimized ones (like it's now.).

[mp911de]

Spring Data converters are now in place.

[jdmwood]

Can you file a ticket in https://github.com/pgjdbc/r2dbc-postgresql asking for an optimization switch for JSON? Probably the same would make sense for Clob and Blob in a way that the driver returns either detached objects (should be the default) or attached/memory-optimized ones (like it's now.).

@mp911de Sorry, was this targeted at me? Do you still need this?

[mp911de]

I just created pgjdbc/r2dbc-postgresql#330. So everything is done here.

[ajeans]

@mp911de sorry for commenting on a closed issue, but I am trying to get Postgres Json / Jsonb columns converted to String rather than io.r2dbc.postgresql.codec.Json

This fails on insertion with

2021-01-05 09:23:34.777 DEBUG 77612 --- [tor-tcp-epoll-2] o.s.r2dbc.core.DefaultDatabaseClient     : Executing SQL statement [INSERT INTO onboarding_case (organization, workflow, details) VALUES ($1, $2, $3)]
2021-01-05 09:23:34.795  WARN 77612 --- [tor-tcp-epoll-2] i.r.p.client.ReactorNettyClient          : Error: SEVERITY_LOCALIZED=ERROR, SEVERITY_NON_LOCALIZED=ERROR, CODE=42804, MESSAGE=column "details" is of type jsonb but expression is of type character varying, HINT=You will need to rewrite or cast the expression., POSITION=79, FILE=parse_target.c, LINE=590, ROUTINE=transformAssignedExpr

Looking at the commit 4506fb6 it seems that the converters are only implemented on the reading side, not on the writing side, so this is kind of expected.

Is my analysis correct? Should I open another issue on this topic?