Re: fd.c: flush data problems on osx

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Stas Kelvich <s(dot)kelvich(at)postgrespro(dot)ru>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: fd.c: flush data problems on osx
Date: 2016-04-13 22:18:31
Message-ID: 20160413221831.prrqzypeq2ofezaj@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2016-04-13 18:09:18 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On 2016-04-13 17:44:41 -0400, Tom Lane wrote:
> >> fd.c tracks seek position for open files. I'm not sure that that
> >> function can get called with amount == 0, but if it did, the caller
> >> would certainly not be expecting the file position to change.
>
> > Ok, fair enough. (And no, it should currently be never called that way)
>
> BTW, I just noticed another issue here, which is that FileWriteback
> and the corresponding smgr routines are declared with bogusly narrow
> "amount" arguments --- eg, it's silly that FileWriteback only takes
> an int amount.

Well, I modeled it after the nearby routines (like FileRead), which all
only take an amount in int. Now there might be less reason to read a lot
of data at once, than to flush large amounts; but it still didn't seem
necessary to break with the rest of the functions in the file.

> I think this code could be actively broken for relation segment sizes
> exceeding 2GB, and even if it isn't, we should define the functions
> correctly the first time.

I don't think it's actively problematic, ->max_pending (and thus
nr_pending) is limited by #define WRITEBACK_MAX_PENDING_FLUSHES 256
(although I wondered whether we should increase the limit a bit
further). Even with a blocksize of 32768, that's pretty far away from
exceeding INT_MAX.

> Will fix the function definitions, but I'm kind of wondering exactly how
> many times the inner loop in IssuePendingWritebacks() could possibly
> iterate ...

WRITEBACK_MAX_PENDING_FLUSHES/256, due to the limitation mentioned
above.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Lenski 2016-04-13 22:48:57 Re: [HACKERS] sign function with INTERVAL?
Previous Message Michael Paquier 2016-04-13 22:10:39 Re: Missing PG_INT32_MIN in numutils.c