On 2019-Nov-20, Antonin Houska wrote:
> Alvaro Herrera wrote:
> > What is logical_read_local_xlog_page all about? Seems useless. Let's
> > get rid of it.
>
> It seems so. Should I post a patch for that?
No need .. it was simple enough. Just pushed it.
Thanks
--
Álvaro Herrera
Alvaro Herrera wrote:
> On 2019-Nov-25, Antonin Houska wrote:
>
> > Alvaro Herrera wrote:
>
> > > I see no reason to leave ws_off. We can move that to XLogReaderState; I
> > > did that here. We also need the offset in WALReadError, though, so I
> > > added it there too. Conceptually it
On 2019-Nov-25, Antonin Houska wrote:
> Alvaro Herrera wrote:
> > I see no reason to leave ws_off. We can move that to XLogReaderState; I
> > did that here. We also need the offset in WALReadError, though, so I
> > added it there too. Conceptually it seems clearer to me this way.
> >
> >
Alvaro Herrera wrote:
> On 2019-Nov-22, Antonin Houska wrote:
>
> > As I pointed out in
> >
> > https://www.postgresql.org/message-id/88183.1574261429%40antos
> >
> > seg.ws_off only replaced readOff in XLogReaderState. So we should only
> > update
> > ws_off where readOff was updated before
On Fri, Nov 22, 2019 at 07:56:32PM -0300, Alvaro Herrera wrote:
> I see no reason to leave ws_off. We can move that to XLogReaderState; I
> did that here. We also need the offset in WALReadError, though, so I
> added it there too. Conceptually it seems clearer to me this way.
Yeah, that seems
On 2019-Nov-22, Antonin Houska wrote:
> As I pointed out in
>
> https://www.postgresql.org/message-id/88183.1574261429%40antos
>
> seg.ws_off only replaced readOff in XLogReaderState. So we should only update
> ws_off where readOff was updated before commit 709d003. This does happen in
>
Alvaro Herrera wrote:
> On 2019-Nov-22, Michael Paquier wrote:
>
> > On Fri, Nov 22, 2019 at 10:35:51AM -0300, Alvaro Herrera wrote:
> > > FWIW I think the new code is buggy because it doesn't seem to be setting
> > > ws_off, so I suppose the optimization in ReadPageInternal to skip
> > >
On 2019-Nov-22, Michael Paquier wrote:
> On Fri, Nov 22, 2019 at 10:35:51AM -0300, Alvaro Herrera wrote:
> > FWIW I think the new code is buggy because it doesn't seem to be setting
> > ws_off, so I suppose the optimization in ReadPageInternal to skip
> > reading the page when it's already the
On Fri, Nov 22, 2019 at 10:35:51AM -0300, Alvaro Herrera wrote:
> FWIW I think the new code is buggy because it doesn't seem to be setting
> ws_off, so I suppose the optimization in ReadPageInternal to skip
> reading the page when it's already the page we have is not hit, except
> for the first
On 2019-Nov-22, Antonin Houska wrote:
> I thought that in [1] you try discourage me from using pg_pread(), but now it
> seems to be the opposite. Ideally I'd like to see no overhead added by my
> patch at all, but the code simplicity should matter too.
FWIW I think the new code is buggy because
Michael Paquier wrote:
> On Thu, Nov 21, 2019 at 05:05:50PM +0900, Michael Paquier wrote:
> > And with WAL segments at 1MB, I was seeing quite a slowdown with the
> > patch... Then I have done an extra test with pg_waldump with the
> > segments generated previously with the output redirected to
On Fri, Nov 22, 2019 at 12:49:33AM -0300, Alvaro Herrera wrote:
> Yes :-) hopefully next week. Thanks for reviewing.
Thanks, I am switching the entry as ready for committer then. Please
note that the latest patch series have a conflict at the top of
walsender.c easy enough to resolve, and that
On 2019-Nov-22, Michael Paquier wrote:
> Alvaro, you are marked as a committer of this CF entry. Are you
> planning to look at it again? Sorry for the delay from my side.
Yes :-) hopefully next week. Thanks for reviewing.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
On Thu, Nov 21, 2019 at 05:05:50PM +0900, Michael Paquier wrote:
> And with WAL segments at 1MB, I was seeing quite a slowdown with the
> patch... Then I have done an extra test with pg_waldump with the
> segments generated previously with the output redirected to /dev/null.
> Going through 512
On Wed, Nov 20, 2019 at 03:50:29PM +0100, Antonin Houska wrote:
> Thus the use of pg_pread() makes the code quite a bit simpler, so I
> re-introduced it. If you decide that an explicit lseek() should be used yet,
> just let me know.
Skimming through the code, it looks like in a good state. The
Alvaro Herrera wrote:
> On 2019-Nov-12, Antonin Houska wrote:
>
> > ok, the next version uses explicit lseek(). Maybe the fact that XLOG is
> > mostly
> > read sequentially (i.e. without frequent seeks) is the reason pread() has't
> > been adopted so far.
>
> I don't quite understand why you
On Mon, Nov 18, 2019 at 09:29:03PM +0900, Michael Paquier wrote:
> Now, read() > pread() > read()+lseek(), and we don't actually need to
> seek into the file for all the cases where we read a WAL page. And on
> a platform which uses the fallback implementation, this increases the
> number of
On Fri, Nov 15, 2019 at 06:41:02PM -0300, Alvaro Herrera wrote:
> I don't quite understand why you backed off from switching to pread. It
> seemed a good change to me.
>
> [...]
>
> Having seek/open be a boolean "xlr_seek" seems a bit weird. Changed to
> an "operation" enum. (Maybe if we go
BTW ... contrib/test_decoding fails with this patch.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index 006c6298c9..e7a504c304 100644
---
On 2019-Nov-12, Antonin Houska wrote:
> ok, the next version uses explicit lseek(). Maybe the fact that XLOG is mostly
> read sequentially (i.e. without frequent seeks) is the reason pread() has't
> been adopted so far.
I don't quite understand why you backed off from switching to pread. It
Michael Paquier wrote:
> On Mon, Nov 11, 2019 at 04:25:56PM +0100, Antonin Houska wrote:
> >> On Fri, Oct 04, 2019 at 12:11:11PM +0200, Antonin Houska wrote:
> >> Your patch removes all the three optional lseek() calls which can
> >> happen in a segment. Am I missing something but isn't that
On Mon, Nov 11, 2019 at 04:25:56PM +0100, Antonin Houska wrote:
>> On Fri, Oct 04, 2019 at 12:11:11PM +0200, Antonin Houska wrote:
> + /*
> +* Update read state information.
> +*
> +* If XLogRead() is was called by ->read_page, it should have updated
> the
> +
Michael Paquier wrote:
> On Fri, Oct 04, 2019 at 12:11:11PM +0200, Antonin Houska wrote:
> > This is the next version.
>
> So... These are the two last bits to look at, reducing a bit the code
> size:
> 5 files changed, 396 insertions(+), 419 deletions(-)
>
> And what this patch set does is
On Fri, Oct 04, 2019 at 12:11:11PM +0200, Antonin Houska wrote:
> This is the next version.
So... These are the two last bits to look at, reducing a bit the code
size:
5 files changed, 396 insertions(+), 419 deletions(-)
And what this patch set does is to refactor the routines we use now in
This is the next version.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
>From 01f5cc8b0c1e6133e16242ec99957a78551058a7 Mon Sep 17 00:00:00 2001
From: Antonin Houska
Date: Fri, 4 Oct 2019 12:07:22 +0200
Subject: [PATCH 1/2] Use only xlogreader.c:XLogRead()
The implementations in
Kyotaro Horiguchi wrote:
> At Tue, 01 Oct 2019 08:28:03 +0200, Antonin Houska wrote
> in <2188.1569911283@antos>
> > Kyotaro Horiguchi wrote:
> > > > The problem of walsender.c is that its implementation of XLogRead()
> > > > does not
> > > > care about the TLI of the previous read. If the
At Tue, 01 Oct 2019 08:28:03 +0200, Antonin Houska wrote in
<2188.1569911283@antos>
> Kyotaro Horiguchi wrote:
> > > > XLogRead() tests for NULL so it should not crash but I don't insist on
> > > > doing
> > > > it this way. XLogRead() actually does not have to care whether the "open
> > > >
Kyotaro Horiguchi wrote:
> At Sat, 28 Sep 2019 15:00:35 +0200, Antonin Houska wrote
> in <9236.1569675635@antos>
> > Antonin Houska wrote:
> >
> > > Alvaro Herrera wrote:
> > >
> > > > BTW that tli_p business to the openSegment callback is horribly
> > > > inconsistent. Some callers
Hello,
At Sat, 28 Sep 2019 15:00:35 +0200, Antonin Houska wrote in
<9236.1569675635@antos>
> Antonin Houska wrote:
>
> > Alvaro Herrera wrote:
> >
> > > BTW that tli_p business to the openSegment callback is horribly
> > > inconsistent. Some callers accept a NULL tli_p, others will
Antonin Houska wrote:
> Alvaro Herrera wrote:
>
> > BTW that tli_p business to the openSegment callback is horribly
> > inconsistent. Some callers accept a NULL tli_p, others will outright
> > crash, even though the API docs say that the callback must determine the
> > timeline. This is made
Tom Lane wrote:
> Alvaro Herrera writes:
> > On 2019-Sep-27, Antonin Houska wrote:
> >>> You placed the errinfo in XLogRead's stack rather than its callers' ...
> >>> I don't think that works, because as soon as XLogRead returns that
> >>> memory is no longer guaranteed to exist.
>
> >> I was
Alvaro Herrera wrote:
> BTW that tli_p business to the openSegment callback is horribly
> inconsistent. Some callers accept a NULL tli_p, others will outright
> crash, even though the API docs say that the callback must determine the
> timeline. This is made more complicated by us having the
Alvaro Herrera writes:
> On 2019-Sep-27, Antonin Houska wrote:
>>> You placed the errinfo in XLogRead's stack rather than its callers' ...
>>> I don't think that works, because as soon as XLogRead returns that
>>> memory is no longer guaranteed to exist.
>> I was aware of this problem, therefore
On 2019-Sep-27, Antonin Houska wrote:
> > You placed the errinfo in XLogRead's stack rather than its callers' ...
> > I don't think that works, because as soon as XLogRead returns that
> > memory is no longer guaranteed to exist.
>
> I was aware of this problem, therefore I defined the field as
Alvaro Herrera wrote:
> On 2019-Sep-26, Antonin Houska wrote:
> You placed the errinfo in XLogRead's stack rather than its callers' ...
> I don't think that works, because as soon as XLogRead returns that
> memory is no longer guaranteed to exist.
I was aware of this problem, therefore I
On 2019-Sep-26, Antonin Houska wrote:
> One comment on the remaining part of the series:
>
> Before this refactoring, the walsender.c:XLogRead() function contained these
> lines
>
>/*
> * After reading into the buffer, check that what we read was valid.
> We do
> * this
Alvaro Herrera wrote:
> On 2019-Sep-24, Antonin Houska wrote:
>
> > Alvaro Herrera wrote:
>
> > > If you don't have any strong dislikes for these changes, I'll push this
> > > part and let you rebase the remains on top.
> >
> > No objections here.
>
> oK, pushed. Please rebase the other
On 2019-Sep-24, Antonin Houska wrote:
> Alvaro Herrera wrote:
> > If you don't have any strong dislikes for these changes, I'll push this
> > part and let you rebase the remains on top.
>
> No objections here.
oK, pushed. Please rebase the other parts.
I made one small adjustment: in
Alvaro Herrera wrote:
> I spent a couple of hours on this patchset today. I merged 0001 and
> 0002, and decided the result was still messier than I would have liked,
> so I played with it a bit more -- see attached. I think this is
> committable, but I'm afraid it'll cause quite a few
I spent a couple of hours on this patchset today. I merged 0001 and
0002, and decided the result was still messier than I would have liked,
so I played with it a bit more -- see attached. I think this is
committable, but I'm afraid it'll cause quite a few conflicts with the
rest of your series.
On 2019-Sep-23, Alvaro Herrera wrote:
> I spent a couple of hours on this patchset today. I merged 0001 and
> 0002, and decided the result was still messier than I would have liked,
> so I played with it a bit more -- see attached.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
Alvaro Herrera wrote:
> I was confused by the struct name XLogSegment -- the struct is used to
> represent a WAL segment while it's kept open, rather than just a WAL
> segment in abstract. Also, now that we've renamed everything to use the
> term WAL, it seems wrong to use the name XLog for new
I was confused by the struct name XLogSegment -- the struct is used to
represent a WAL segment while it's kept open, rather than just a WAL
segment in abstract. Also, now that we've renamed everything to use the
term WAL, it seems wrong to use the name XLog for new structs. I
propose the name
Alvaro Herrera wrote:
> Hi Antonin, could you please rebase again?
Attached.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
>From 7b15d7bbdae13c743ddeae10b8ff79e9b02d8243 Mon Sep 17 00:00:00 2001
From: Antonin Houska
Date: Mon, 9 Sep 2019 11:53:54 +0200
Subject: [PATCH 1/4]
Hi Antonin, could you please rebase again?
Thanks
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Pushed 0001.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Thomas Munro wrote:
> On Tue, May 21, 2019 at 9:12 PM Antonin Houska wrote:
> > Robert Haas wrote:
> > > It seems to me that it's better to unwind the stack i.e. have the
> > > function return the error information to the caller and let the caller
> > > do as it likes.
> >
> > Thanks for a
On Tue, May 21, 2019 at 9:12 PM Antonin Houska wrote:
> Robert Haas wrote:
> > It seems to me that it's better to unwind the stack i.e. have the
> > function return the error information to the caller and let the caller
> > do as it likes.
>
> Thanks for a hint. The next version tries to do
Robert Haas wrote:
> It seems to me that it's better to unwind the stack i.e. have the
> function return the error information to the caller and let the caller
> do as it likes.
Thanks for a hint. The next version tries to do that.
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
On Mon, May 6, 2019 at 2:21 PM Alvaro Herrera wrote:
> On 2019-May-06, Robert Haas wrote:
> > On Thu, May 2, 2019 at 12:18 PM Antonin Houska wrote:
> > > The next version of the patch is attached.
> >
> > I don't think any of this looks acceptable:
>
> I agree. I inteded to suggest upthread to
On 2019-May-06, Robert Haas wrote:
> On Thu, May 2, 2019 at 12:18 PM Antonin Houska wrote:
> > The next version of the patch is attached.
>
> I don't think any of this looks acceptable:
I agree. I inteded to suggest upthread to pass an additional argument
to XLogRead, which is a function that
On Thu, May 2, 2019 at 12:18 PM Antonin Houska wrote:
> The next version of the patch is attached.
I don't think any of this looks acceptable:
+#ifndef FRONTEND
+/*
+ * Backend should have wal_segment_size variable initialized, segsize is not
+ * used.
+ */
+#define XLogFileNameCommon(tli, num,
Antonin Houska wrote:
> Alvaro Herrera wrote:
>
> > Not sure about the walsize; maybe it can be a member in XLogReadPos, and
> > given to XLogReadInitPos()? (Maybe rename XLogReadPos as
> > XLogReadContext or something like that, indicating it's not just the
> > read position.)
>
> As
Alvaro Herrera wrote:
> I agree that xlog reading is pretty messy.
>
> I think ifdef'ing the way XLogRead reports errors is not great. Maybe
> we can pass a function pointer that is to be called in case of errors?
I'll try a bit harder to evaluate the existing approaches to report the same
Michael Paquier wrote:
> On Fri, Apr 12, 2019 at 12:27:11PM +0900, Kyotaro HORIGUCHI wrote:
> > This patch changes XLogRead to allow using other than
> > BasicOpenFile to open a segment, and use XLogReaderState.private
> > to hold a new struct XLogReadPos for the segment reader. The new
> >
Kyotaro HORIGUCHI wrote:
> Hello.
>
> At Thu, 11 Apr 2019 18:05:42 +0200, Antonin Houska wrote
> in <14984.1554998...@spoje.net>
> > While working on the instance encryption I found it annoying to apply
> > decyption of XLOG page to three different functions. Attached is a patch
> > that
> >
On 2019-Apr-11, Antonin Houska wrote:
> While working on the instance encryption I found it annoying to apply
> decyption of XLOG page to three different functions. Attached is a patch that
> tries to merge them all into one function, XLogRead(). The existing
> implementations differ in the way
On Fri, Apr 12, 2019 at 2:06 AM Antonin Houska wrote:
> While working on the instance encryption I found it annoying to apply
> decyption of XLOG page to three different functions. Attached is a patch
> that
> tries to merge them all into one function, XLogRead(). The existing
> implementations
On Fri, Apr 12, 2019 at 12:27:11PM +0900, Kyotaro HORIGUCHI wrote:
> This patch changes XLogRead to allow using other than
> BasicOpenFile to open a segment, and use XLogReaderState.private
> to hold a new struct XLogReadPos for the segment reader. The new
> struct is heavily duplicated with
Hello.
At Thu, 11 Apr 2019 18:05:42 +0200, Antonin Houska wrote in
<14984.1554998...@spoje.net>
> While working on the instance encryption I found it annoying to apply
> decyption of XLOG page to three different functions. Attached is a patch that
> tries to merge them all into one function,
While working on the instance encryption I found it annoying to apply
decyption of XLOG page to three different functions. Attached is a patch that
tries to merge them all into one function, XLogRead(). The existing
implementations differ in the way new segment is opened. So I added a pointer
to
62 matches
Mail list logo
