public inbox for [email protected]
help / color / mirror / Atom feedFrom: Antonin Houska <[email protected]>
To: [email protected]
Subject: Re: Slow I/O can break throttling of base backup
Date: Fri, 16 Dec 2016 11:24:30 +0100
Message-ID: <27929.1481883870@localhost> (raw)
In-Reply-To: <10526.1481818893@localhost>
References: <CABUevEy_-e0YvL4ayoX8bH_Ja9w+BHoP6jUgdxZuG2nEj3uAfQ@mail.gmail.com>
<10526.1481818893@localhost>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgsql-hackers>
Antonin Houska <[email protected]> wrote:
> It seems to be my bug. I'll check tomorrow.
I could reproduce the problem by adding sufficient sleep time to the
loop.
> Magnus Hagander <[email protected]> wrote:
>> I wonder if the else if (sleep > 0) at the bottom of throttle() should just
>> be a simple else and always run, resetting last_throttle?
I agree. In fact, I could simplify the code even more.
Since (elapsed + sleep) almost equals to GetCurrentIntegerTimestamp(), we can
use the following statement unconditionally (I think I tried too hard to avoid
calling GetCurrentIntegerTimestamp too often in the original patch):
throttled_last = GetCurrentIntegerTimestamp();
Thus we can also get rid of the "else" branch that clears both "sleep" and
"wait_result".
(The patch contains minor comment refinement that I found useful when seeing
the code after years.)
--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[text/x-diff] throttling.patch (1.4K, ../27929.1481883870@localhost/2-throttling.patch)
download | inline diff:
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
new file mode 100644
index ffc7e58..40b3c11
*** a/src/backend/replication/basebackup.c
--- b/src/backend/replication/basebackup.c
*************** throttle(size_t increment)
*** 1370,1395 ****
if (wait_result & WL_LATCH_SET)
CHECK_FOR_INTERRUPTS();
}
- else
- {
- /*
- * The actual transfer rate is below the limit. A negative value
- * would distort the adjustment of throttled_last.
- */
- wait_result = 0;
- sleep = 0;
- }
/*
! * Only a whole multiple of throttling_sample was processed. The rest will
! * be done during the next call of this function.
*/
throttling_counter %= throttling_sample;
! /* Once the (possible) sleep has ended, new period starts. */
! if (wait_result & WL_TIMEOUT)
! throttled_last += elapsed + sleep;
! else if (sleep > 0)
! /* Sleep was necessary but might have been interrupted. */
! throttled_last = GetCurrentIntegerTimestamp();
}
--- 1370,1385 ----
if (wait_result & WL_LATCH_SET)
CHECK_FOR_INTERRUPTS();
}
/*
! * As we work with integers, only whole multiple of throttling_sample was
! * processed. The rest will be done during the next call of this function.
*/
throttling_counter %= throttling_sample;
! /*
! * Time interval for the remaining amount and possible next increments
! * starts now.
! */
! throttled_last = GetCurrentIntegerTimestamp();
}
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Slow I/O can break throttling of base backup
In-Reply-To: <27929.1481883870@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox