Skip to content

Commit

Permalink
Lazy detoast, working in PG 11beta3, 10, 9.6.
Browse files Browse the repository at this point in the history
Introduce lazy detoasting, where if the value underlying a readable
SQLXML instance is found not to be fully detoasted at the time of
construction, it's allowed to stay that way until the contents are
actually needed, as discussed in

https://www.postgresql.org/message-id/1c64290b-b729-eeab-219e-1577a12e9b5a%40anastigmatix.net

(and above and below in the same message thread), implementing the
"just use the oldest one" snapshot selection strategy from

https://www.postgresql.org/message-id/8ca78589-734b-f904-1cc5-007eeb5d4737%40anastigmatix.net

The implementation passes the following test inspired by Andrew Gierth in
https://www.postgresql.org/message-id/877eovbjc3.fsf%40news-spur.riddles.org.uk

CREATE TABLE t(x xml);
BEGIN READ WRITE, ISOLATION LEVEL READ COMMITTED;
/*
 * In other session: INSERT INTO t(x)
 * SELECT table_to_xml('pg_operator', true, false, '');
 */
SELECT javatest.echoxmlparameter(x, 0, 5) FROM t; -- 0 => stash x
/*
 * In other session: DELETE FROM t;
 * VACUUM t;
 */
SELECT javatest.echoxmlparameter(null, 5, 5); -- null => unstash
COMMIT;

And, indeed, the same test is made to fail by commenting out the
snapshot registrations/unregistrations in VarlenaWrapper.c, so
they are performing the expected valuable service. Without them:
ERROR:  missing chunk number 0 for toast value 16716 in pg_toast_16708
  • Loading branch information
jcflack committed Aug 12, 2018
1 parent 925cec2 commit 1a5caf1
Show file tree
Hide file tree
Showing 4 changed files with 311 additions and 24 deletions.
11 changes: 10 additions & 1 deletion pljava-so/src/main/c/DualState.c
Expand Up @@ -112,7 +112,16 @@ static void resourceReleaseCB(ResourceReleasePhase phase,
StaticAssertStmt(sizeof p2l.ptrVal <= sizeof p2l.longVal,
"Pointer will not fit in long on this platform");

if ( RESOURCE_RELEASE_AFTER_LOCKS != phase )
/*
* The way ResourceOwnerRelease is implemented, callbacks to loadable
* modules (like us!) happen /after/ all of the built-in releasey actions
* for a particular phase. So, by looking for RESOURCE_RELEASE_LOCKS here,
* we actually end up executing after all the built-in lock-related stuff
* has been released, but before any of the built-in stuff released in the
* RESOURCE_RELEASE_AFTER_LOCKS phase. Which, at least for the currently
* implemented DualState subclasses, is about the right time.
*/
if ( RESOURCE_RELEASE_LOCKS != phase )
return;

p2l.longVal = 0L;
Expand Down
204 changes: 194 additions & 10 deletions pljava-so/src/main/c/VarlenaWrapper.c
Expand Up @@ -11,6 +11,13 @@
* Chapman Flack
*/

#include <postgres.h>
#include <access/tuptoaster.h>
#include <utils/datum.h>
#include <utils/snapshot.h>
#include <utils/snapmgr.h>

#include "org_postgresql_pljava_internal_VarlenaWrapper_Input_State.h"
#include "org_postgresql_pljava_internal_VarlenaWrapper_Output_State.h"
#include "pljava/VarlenaWrapper.h"
#include "pljava/DualState.h"
Expand All @@ -35,6 +42,8 @@ static jmethodID s_VarlenaWrapper_Input_init;

static jmethodID s_VarlenaWrapper_Output_init;

static jfieldID s_VarlenaWrapper_Input_State_varlena;

/*
* For VarlenaWrapper.Output, define a dead-simple "expanded object" format
* consisting of linked allocated blocks, so if a long value is being written,
Expand Down Expand Up @@ -121,32 +130,74 @@ jobject pljava_VarlenaWrapper_Input(
jobject dbb;
MemoryContext mc;
MemoryContext prevcxt;
struct varlena *copy;
struct varlena *vl;
Ptr2Long p2lro;
Ptr2Long p2lcxt;
Ptr2Long p2lpin;
Ptr2Long p2ldatum;
Size parked;
Size actual;
Snapshot pin = NULL;

vl = (struct varlena *) DatumGetPointer(d);
if ( VARATT_IS_EXTERNAL_INDIRECT(vl) ) /* at most once; can't be nested */
{
struct varatt_indirect redirect;
VARATT_EXTERNAL_GET_POINTER(redirect, vl);
vl = (struct varlena *)redirect.pointer;
d = PointerGetDatum(vl);
}

parked = VARSIZE_ANY(vl);
actual = toast_raw_datum_size(d) - VARHDRSZ;

mc = AllocSetContextCreate(parent, "PL/Java VarlenaWrapper.Input",
ALLOCSET_START_SMALL_SIZES);

prevcxt = MemoryContextSwitchTo(mc);
copy = PG_DETOAST_DATUM_COPY(d);

if ( actual < 4096 || (actual >> 1) < parked )
goto justDetoastEagerly;
if ( VARATT_IS_EXTERNAL_EXPANDED(vl) )
goto justDetoastEagerly;
if ( VARATT_IS_EXTERNAL_ONDISK(vl) )
{
pin = GetOldestSnapshot();
if ( NULL == pin )
goto justDetoastEagerly;
pin = RegisterSnapshotOnOwner(pin, ro);
}

/* parkAndDetoastLazily: */
vl = (struct varlena *) DatumGetPointer(datumCopy(d, false, -1));
dbb = NULL;
goto constructResult;

justDetoastEagerly:
vl = PG_DETOAST_DATUM_COPY(d);
parked = actual + VARHDRSZ;
dbb = JNI_newDirectByteBuffer(VARDATA(vl), actual);

constructResult:
MemoryContextSwitchTo(prevcxt);

p2lro.longVal = 0L;
p2lcxt.longVal = 0L;
p2lpin.longVal = 0L;
p2ldatum.longVal = 0L;

p2lro.ptrVal = ro;
p2lcxt.ptrVal = mc;
p2ldatum.ptrVal = copy;

dbb = JNI_newDirectByteBuffer(VARDATA(copy), VARSIZE_ANY_EXHDR(copy));
p2lpin.ptrVal = pin;
p2ldatum.ptrVal = vl;

vr = JNI_newObjectLocked(s_VarlenaWrapper_Input_class,
s_VarlenaWrapper_Input_init, pljava_DualState_key(),
p2lro.longVal, p2lcxt.longVal, p2ldatum.longVal, dbb);
JNI_deleteLocalRef(dbb);
p2lro.longVal, p2lcxt.longVal, p2lpin.longVal, p2ldatum.longVal,
parked, actual, dbb);

if ( NULL != dbb )
JNI_deleteLocalRef(dbb);

return vr;
}
Expand Down Expand Up @@ -298,7 +349,26 @@ static void VOS_flatten_into(ExpandedObjectHeader *eohptr,
void pljava_VarlenaWrapper_initialize(void)
{
jclass clazz;
JNINativeMethod methods[] =
JNINativeMethod methodsIn[] =
{
{
"_unregisterSnapshot",
"(JJ)V",
Java_org_postgresql_pljava_internal_VarlenaWrapper_00024Input_00024State__1unregisterSnapshot
},
{
"_detoast",
"(JJJJ)Ljava/nio/ByteBuffer;",
Java_org_postgresql_pljava_internal_VarlenaWrapper_00024Input_00024State__1detoast
},
{
"_fetch",
"(JJ)J",
Java_org_postgresql_pljava_internal_VarlenaWrapper_00024Input_00024State__1fetch
},
{ 0, 0, 0 }
};
JNINativeMethod methodsOut[] =
{
{
"_nextBuffer",
Expand All @@ -321,7 +391,7 @@ void pljava_VarlenaWrapper_initialize(void)
s_VarlenaWrapper_Input_init = PgObject_getJavaMethod(
s_VarlenaWrapper_Input_class, "<init>",
"(Lorg/postgresql/pljava/internal/DualState$Key;"
"JJJLjava/nio/ByteBuffer;)V");
"JJJJJJLjava/nio/ByteBuffer;)V");

s_VarlenaWrapper_Output_init = PgObject_getJavaMethod(
s_VarlenaWrapper_Output_class, "<init>",
Expand All @@ -332,12 +402,126 @@ void pljava_VarlenaWrapper_initialize(void)
s_VarlenaWrapper_class, "adopt",
"(Lorg/postgresql/pljava/internal/DualState$Key;)J");

clazz = (jclass)JNI_newGlobalRef(PgObject_getJavaClass(
"org/postgresql/pljava/internal/VarlenaWrapper$Input$State"));

PgObject_registerNatives2(clazz, methodsIn);

s_VarlenaWrapper_Input_State_varlena = PgObject_getJavaField(
clazz, "m_varlena", "J");

JNI_deleteLocalRef(clazz);

clazz = (jclass)JNI_newGlobalRef(PgObject_getJavaClass(
"org/postgresql/pljava/internal/VarlenaWrapper$Output$State"));
PgObject_registerNatives2(clazz, methods);

PgObject_registerNatives2(clazz, methodsOut);

JNI_deleteLocalRef(clazz);
}

/*
* Class: org_postgresql_pljava_internal_VarlenaWrapper_Input_State
* Method: _unregisterSnapshot
* Signature: (JJ)V
*/
JNIEXPORT void JNICALL
Java_org_postgresql_pljava_internal_VarlenaWrapper_00024Input_00024State__1unregisterSnapshot
(JNIEnv *env, jobject _this, jlong snapshot, jlong ro)
{
BEGIN_NATIVE_NO_ERRCHECK
Ptr2Long p2lsnap;
Ptr2Long p2lro;
p2lsnap.longVal = snapshot;
p2lro.longVal = ro;
UnregisterSnapshotFromOwner(p2lsnap.ptrVal, p2lro.ptrVal);
END_NATIVE
}

/*
* Class: org_postgresql_pljava_internal_VarlenaWrapper_Input_State
* Method: _detoast
* Signature: (JJJJ)Ljava/nio/ByteBuffer;
*/
JNIEXPORT jobject JNICALL
Java_org_postgresql_pljava_internal_VarlenaWrapper_00024Input_00024State__1detoast
(JNIEnv *env, jobject _this, jlong vl, jlong cxt, jlong snap, jlong resOwner)
{
Ptr2Long p2lvl;
Ptr2Long p2lcxt;
Ptr2Long p2lsnap;
Ptr2Long p2lro;
Ptr2Long p2ldetoasted;
struct varlena *detoasted;
MemoryContext prevcxt;
jobject dbb;

BEGIN_NATIVE_NO_ERRCHECK

p2lvl.longVal = vl;
p2lcxt.longVal = cxt;
p2lsnap.longVal = snap;
p2lro.longVal = resOwner;

prevcxt = MemoryContextSwitchTo((MemoryContext)p2lcxt.ptrVal);

detoasted = PG_DETOAST_DATUM_COPY(PointerGetDatum(p2lvl.ptrVal));
p2ldetoasted.longVal = 0L;
p2ldetoasted.ptrVal = detoasted;

MemoryContextSwitchTo(prevcxt);

JNI_setLongField(_this,
s_VarlenaWrapper_Input_State_varlena, p2ldetoasted.longVal);
pfree(p2lvl.ptrVal);

if ( 0 != snap )
UnregisterSnapshotFromOwner(p2lsnap.ptrVal, p2lro.ptrVal);

dbb = JNI_newDirectByteBuffer(
VARDATA(detoasted), VARSIZE_ANY_EXHDR(detoasted));

END_NATIVE

return dbb;
}

/*
* Class: org_postgresql_pljava_internal_VarlenaWrapper_Input_State
* Method: _fetch
* Signature: (JJ)J
*
* Assumption: this is only called when a snapshot has been registered (meaning
* the varlena is EXTERNAL_ONDISK) and the snapshot is soon to be unregistered.
* All that's needed is to 'fetch' the representation from disk, in case the
* toast rows could be subject to vacuuming after the snapshot is unregistered.
* A fetch is not a full detoast; if what's fetched is compressed, it stays
* compressed. This method does not need to unregister the snapshot, as that
* will happen soon anyway. It does pfree the toast pointer.
*/
JNIEXPORT jlong JNICALL Java_org_postgresql_pljava_internal_VarlenaWrapper_00024Input_00024State__1fetch
(JNIEnv *env, jobject _this, jlong varlena, jlong memContext)
{
Ptr2Long p2lvl;
Ptr2Long p2lcxt;
MemoryContext prevcxt;
struct varlena *fetched;

BEGIN_NATIVE_NO_ERRCHECK;
p2lvl.longVal = varlena;
p2lcxt.longVal = memContext;

prevcxt = MemoryContextSwitchTo((MemoryContext) p2lcxt.ptrVal);
fetched = heap_tuple_fetch_attr((struct varlena *)p2lvl.ptrVal);
pfree(p2lvl.ptrVal);
p2lvl.longVal = 0L;
p2lvl.ptrVal = fetched;
MemoryContextSwitchTo(prevcxt);

END_NATIVE;
return p2lvl.longVal;
}

/*
* Class: org_postgresql_pljava_internal_VarlenaWrapper_Output_State
* Method: _nextBuffer
Expand Down
12 changes: 10 additions & 2 deletions pljava/src/main/java/org/postgresql/pljava/internal/DualState.java
Expand Up @@ -101,7 +101,7 @@ public abstract class DualState<T> extends WeakReference<T>
* Pointer value of the {@code ResourceOwner} this instance belongs to,
* if any.
*/
private final long m_resourceOwner;
protected final long m_resourceOwner;

/**
* Check that a cookie is valid, throwing an unchecked exception otherwise.
Expand Down Expand Up @@ -291,6 +291,13 @@ public String toString(Object o)
* @param resourceOwner Pointer value identifying the resource owner being
* released. Calls can be received for resource owners to which no instances
* here have been registered.
*<p>
* Some state subclasses may have their nativeStateReleased methods called
* from Java code, when it is clear the native state is no longer needed in
* Java. That doesn't remove the state instance from s_liveInstances though,
* so it will still eventually be seen by this loop and efficiently removed
* by the iterator. Hence the nativeStateIsValid test, to avoid invoking
* nativeStateReleased more than once.
*/
private static void resourceOwnerRelease(long resourceOwner)
{
Expand All @@ -303,7 +310,8 @@ private static void resourceOwnerRelease(long resourceOwner)
i.remove();
synchronized ( s )
{
s.nativeStateReleased();
if ( s.nativeStateIsValid() )
s.nativeStateReleased();
}
}
}
Expand Down

0 comments on commit 1a5caf1

Please sign in to comment.