Skip to content

Commit

Permalink
Fix an oversight in format 2: nextlsn
Browse files Browse the repository at this point in the history 
Format 2 (1b0cbac) forgot to add
nextlsn. Format 1 includes this information if include-lsn is provided.
This information can be useful for applications that use LSN position as
a restart point.
  • Loading branch information
@eulerto
eulerto committed Jul 26, 2020
1 parent 3511384 commit 8600d48
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions wal2json.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,10 @@ pg_decode_begin_txn_v2(LogicalDecodingContext *ctx, ReorderBufferTXN *txn)
char *lsn_str = DatumGetCString(DirectFunctionCall1(pg_lsn_out, UInt64GetDatum(txn->final_lsn)));
appendStringInfo(ctx->out, ",\"lsn\":\"%s\"", lsn_str);
pfree(lsn_str);

lsn_str = DatumGetCString(DirectFunctionCall1(pg_lsn_out, UInt64GetDatum(txn->end_lsn)));
appendStringInfo(ctx->out, ",\"nextlsn\":\"%s\"", lsn_str);
pfree(lsn_str);
}

appendStringInfoChar(ctx->out, '}');
Expand Down Expand Up @@ -931,6 +935,10 @@ pg_decode_commit_txn_v2(LogicalDecodingContext *ctx, ReorderBufferTXN *txn,
char *lsn_str = DatumGetCString(DirectFunctionCall1(pg_lsn_out, UInt64GetDatum(commit_lsn)));
appendStringInfo(ctx->out, ",\"lsn\":\"%s\"", lsn_str);
pfree(lsn_str);

lsn_str = DatumGetCString(DirectFunctionCall1(pg_lsn_out, UInt64GetDatum(txn->end_lsn)));
appendStringInfo(ctx->out, ",\"nextlsn\":\"%s\"", lsn_str);
pfree(lsn_str);
}

appendStringInfoChar(ctx->out, '}');
Expand Down

1 comment on commit 8600d48

@dko-slapdash
Copy link

@dko-slapdash dko-slapdash commented on 8600d48 Sep 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we heavily use wal2json, and there are no alternative to it at all (noone supports both insert/update/delete AND pg_logical_emit_message), I typically have lots of questions, hopefully valuable to be added to README's FAQ too or so.

Regarding this particular commit, I wonder if nextlsn is absolutely essential for position tracking, or is it just a "good to have in some cases" thing?

Because in our codebase, I was able to successfully track the position based on just the "transaction commit" message's LSN emitted along with the PG binary replication protocol message (described in this comment: #172 (comment)). To me, it looks like "transaction commit" message has enough information for the proper tracking. I wonder how people use nextlsn in practice in a way which is not available via "transaction commit" LSN.

We anyway need to wait for the "transaction commit" message before storing the LSN locally for tracking purposes, otherwise we risk to lose the data. So why do we want nextlsn in the "transaction begin" message too? (I read the PRs and issues related to nextlsn in v1, but still couldn't figure out the answer to this).

Please sign in to comment.