vacuumlo - use a cursor
vacuumlo is rather simpleminded about dealing with the list of LOs to be
removed - it just fetches them as a straight resultset. For one of my
our this resulted in an out of memory condition. The attached patch
tries to remedy that by using a cursor instead. If this is wanted I will
add it to the next commitfest. The actualy changes are very small - most
of the patch is indentation changes due to the introduction of an extra
loop.
cheers
andrew
Attachments:
vacuumlo-cursor.patchtext/x-patch; name=vacuumlo-cursor.patchDownload
*** a/contrib/vacuumlo/vacuumlo.c
--- b/contrib/vacuumlo/vacuumlo.c
***************
*** 290,362 **** vacuumlo(const char *database, const struct _param * param)
PQclear(res);
buf[0] = '\0';
! strcat(buf, "SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }
- matched = PQntuples(res);
deleted = 0;
! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));
! if (param->verbose)
{
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
}
! if (param->dry_run == 0)
{
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
{
! success = false;
! break;
}
}
else
deleted++;
! }
! else
! deleted++;
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
! {
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
! PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
- PQclear(res);
- PQfinish(conn);
- return -1;
}
- PQclear(res2);
}
}
PQclear(res);
/*
--- 290,389 ----
PQclear(res);
buf[0] = '\0';
! strcat(buf,
! "DECLARE myportal CURSOR WITH HOLD FOR SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "DECLARE CURSOR failed: %s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }
! PQclear(res);
!
! snprintf(buf, BUFSIZE, "FETCH FORWARD " INT64_FORMAT " IN myportal",
! param->transaction_limit > 0 ? param->transaction_limit : 1000);
deleted = 0;
!
! while (1)
{
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
! matched = PQntuples(res);
!
! if (matched <= 0)
{
! /* at end of resultset */
! break;
}
! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));
!
! if (param->verbose)
! {
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
! }
!
! if (param->dry_run == 0)
{
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
! {
! success = false;
! break;
! }
}
+ else
+ deleted++;
}
else
deleted++;
!
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
{
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
}
}
}
+
PQclear(res);
/*
Is there a reason this patch was not applied?
---------------------------------------------------------------------------
On Mon, Nov 12, 2012 at 05:14:57PM -0500, Andrew Dunstan wrote:
vacuumlo is rather simpleminded about dealing with the list of LOs
to be removed - it just fetches them as a straight resultset. For
one of my our this resulted in an out of memory condition. The
attached patch tries to remedy that by using a cursor instead. If
this is wanted I will add it to the next commitfest. The actualy
changes are very small - most of the patch is indentation changes
due to the introduction of an extra loop.cheers
andrew
*** a/contrib/vacuumlo/vacuumlo.c --- b/contrib/vacuumlo/vacuumlo.c *************** *** 290,362 **** vacuumlo(const char *database, const struct _param * param) PQclear(res);buf[0] = '\0';
! strcat(buf, "SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }- matched = PQntuples(res);
deleted = 0;
! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));! if (param->verbose)
{
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
}! if (param->dry_run == 0)
{
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
{
! success = false;
! break;
}
}
else
deleted++;
! }
! else
! deleted++;
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
! {
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
! PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
- PQclear(res);
- PQfinish(conn);
- return -1;
}
- PQclear(res2);
}
}
PQclear(res);/* --- 290,389 ---- PQclear(res);buf[0] = '\0';
! strcat(buf,
! "DECLARE myportal CURSOR WITH HOLD FOR SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "DECLARE CURSOR failed: %s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }
! PQclear(res);
!
! snprintf(buf, BUFSIZE, "FETCH FORWARD " INT64_FORMAT " IN myportal",
! param->transaction_limit > 0 ? param->transaction_limit : 1000);deleted = 0;
!
! while (1)
{
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
! PQfinish(conn);
! return -1;
! }! matched = PQntuples(res);
!
! if (matched <= 0)
{
! /* at end of resultset */
! break;
}! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));
!
! if (param->verbose)
! {
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
! }
!
! if (param->dry_run == 0)
{
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
! {
! success = false;
! break;
! }
}
+ else
+ deleted++;
}
else
deleted++;
!
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
{
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
}
}
}
+
PQclear(res);/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Nobody seemed interested. But I do think it's a good idea still.
cheers
andrew
On 06/29/2013 11:23 AM, Bruce Momjian wrote:
Is there a reason this patch was not applied?
---------------------------------------------------------------------------
On Mon, Nov 12, 2012 at 05:14:57PM -0500, Andrew Dunstan wrote:
vacuumlo is rather simpleminded about dealing with the list of LOs
to be removed - it just fetches them as a straight resultset. For
one of my our this resulted in an out of memory condition. The
attached patch tries to remedy that by using a cursor instead. If
this is wanted I will add it to the next commitfest. The actualy
changes are very small - most of the patch is indentation changes
due to the introduction of an extra loop.cheers
andrew *** a/contrib/vacuumlo/vacuumlo.c --- b/contrib/vacuumlo/vacuumlo.c *************** *** 290,362 **** vacuumlo(const char *database, const struct _param * param) PQclear(res);buf[0] = '\0';
! strcat(buf, "SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }- matched = PQntuples(res);
deleted = 0;
! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));! if (param->verbose)
{
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
}! if (param->dry_run == 0)
{
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
{
! success = false;
! break;
}
}
else
deleted++;
! }
! else
! deleted++;
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
! {
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
! PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
- PQclear(res);
- PQfinish(conn);
- return -1;
}
- PQclear(res2);
}
}
PQclear(res);/* --- 290,389 ---- PQclear(res);buf[0] = '\0';
! strcat(buf,
! "DECLARE myportal CURSOR WITH HOLD FOR SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "DECLARE CURSOR failed: %s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }
! PQclear(res);
!
! snprintf(buf, BUFSIZE, "FETCH FORWARD " INT64_FORMAT " IN myportal",
! param->transaction_limit > 0 ? param->transaction_limit : 1000);deleted = 0;
!
! while (1)
{
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
! PQfinish(conn);
! return -1;
! }! matched = PQntuples(res);
!
! if (matched <= 0)
{
! /* at end of resultset */
! break;
}! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));
!
! if (param->verbose)
! {
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
! }
!
! if (param->dry_run == 0)
{
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
! {
! success = false;
! break;
! }
}
+ else
+ deleted++;
}
else
deleted++;
!
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
{
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
}
}
}
+
PQclear(res);/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Jun 29, 2013 at 11:33:54AM -0400, Andrew Dunstan wrote:
Nobody seemed interested. But I do think it's a good idea still.
Well, if no one replied, and you thought it was a good idea, then it was
a good idea. ;-)
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 06/29/2013 08:35 AM, Bruce Momjian wrote:
On Sat, Jun 29, 2013 at 11:33:54AM -0400, Andrew Dunstan wrote:
Nobody seemed interested. But I do think it's a good idea still.
Well, if no one replied, and you thought it was a good idea, then it was
a good idea. ;-)
I think it is a good idea just of limited use. I only have one customer
that still uses large objects. Which is a shame really as they are more
efficient that bytea.
JD
--
Command Prompt, Inc. - http://www.commandprompt.com/ 509-416-6579
PostgreSQL Support, Training, Professional Services and Development
High Availability, Oracle Conversion, Postgres-XC, @cmdpromptinc
For my dreams of your image that blossoms
a rose in the deeps of my heart. - W.B. Yeats
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 06/29/2013 11:35 AM, Bruce Momjian wrote:
On Sat, Jun 29, 2013 at 11:33:54AM -0400, Andrew Dunstan wrote:
Nobody seemed interested. But I do think it's a good idea still.
Well, if no one replied, and you thought it was a good idea, then it was
a good idea. ;-)
I try not to assume that even if I think it's a good idea it will be
generally wanted unless at least one other person speaks up. But now
that Joshua has I will revive it and add it to the next commitfest.
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Jun 29, 2013 at 12:21:11PM -0400, Andrew Dunstan wrote:
On 06/29/2013 11:35 AM, Bruce Momjian wrote:
On Sat, Jun 29, 2013 at 11:33:54AM -0400, Andrew Dunstan wrote:
Nobody seemed interested. But I do think it's a good idea still.
Well, if no one replied, and you thought it was a good idea, then it was
a good idea. ;-)I try not to assume that even if I think it's a good idea it will be
generally wanted unless at least one other person speaks up. But now
that Joshua has I will revive it and add it to the next commitfest.
Great.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Nov 12, 2012 at 5:14 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
vacuumlo is rather simpleminded about dealing with the list of LOs to be
removed - it just fetches them as a straight resultset. For one of my our
this resulted in an out of memory condition.
Wow, they must have had a ton of LOs. With about 2M entries to pull
from vacuum_l, I observed unpatched vacuumlo using only about 45MB
RES. Still, the idea of using a cursor for the main loop seems like a
reasonable idea.
The attached patch tries to
remedy that by using a cursor instead. If this is wanted I will add it to
the next commitfest. The actualy changes are very small - most of the patch
is indentation changes due to the introduction of an extra loop.
I had some time to review this, some comments about the patch:
1. I see this new compiler warning:
vacuumlo.c: In function ‘vacuumlo’:
vacuumlo.c:306:5: warning: format ‘%lld’ expects argument of type
‘long long int’, but argument 4 has type ‘long int’ [-Wformat]
2. It looks like the the patch causes all the intermediate result-sets
fetched from the cursor to be leaked, rather negating its stated
purpose ;-) The PQclear() call should be moved up into the main loop.
With this fixed, I confirmed that vacuumlo now consumes a negligible
amount of memory when chewing through millions of entries.
3. A few extra trailing whitespaces were added.
4. The FETCH FORWARD count comes from transaction_limit. That seems
like a good-enough guesstimate, but maybe a comment could be added to
rationalize?
Some suggested changes attached with v2 patch (all except #4).
Josh
Attachments:
vacuumlo-cursor.v2.patchapplication/octet-stream; name=vacuumlo-cursor.v2.patchDownload
diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c
new file mode 100644
index 70f7ea7..74bb0d5
*** a/contrib/vacuumlo/vacuumlo.c
--- b/contrib/vacuumlo/vacuumlo.c
*************** vacuumlo(const char *database, const str
*** 290,363 ****
PQclear(res);
buf[0] = '\0';
! strcat(buf, "SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }
- matched = PQntuples(res);
deleted = 0;
! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));
! if (param->verbose)
{
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
}
! if (param->dry_run == 0)
{
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
{
! success = false;
! break;
}
}
else
deleted++;
! }
! else
! deleted++;
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
! {
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
{
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
! PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
PQclear(res2);
- PQclear(res);
- PQfinish(conn);
- return -1;
}
- PQclear(res2);
}
}
- PQclear(res);
/*
* That's all folks!
--- 290,391 ----
PQclear(res);
buf[0] = '\0';
! strcat(buf,
! "DECLARE myportal CURSOR WITH HOLD FOR SELECT lo FROM vacuum_l");
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "DECLARE CURSOR failed: %s", PQerrorMessage(conn));
! PQclear(res);
PQfinish(conn);
return -1;
! }
! PQclear(res);
!
! snprintf(buf, BUFSIZE, "FETCH FORWARD %ld IN myportal",
! param->transaction_limit > 0 ? param->transaction_limit : 1000L);
deleted = 0;
!
! while (1)
{
! res = PQexec(conn, buf);
! if (PQresultStatus(res) != PGRES_TUPLES_OK)
! {
! fprintf(stderr, "Failed to read temp table:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
! matched = PQntuples(res);
! if (matched <= 0)
{
! /* at end of resultset */
! PQclear(res);
! break;
}
! for (i = 0; i < matched; i++)
{
! Oid lo = atooid(PQgetvalue(res, i, 0));
!
! if (param->verbose)
{
! fprintf(stdout, "\rRemoving lo %6u ", lo);
! fflush(stdout);
! }
!
! if (param->dry_run == 0)
! {
! if (lo_unlink(conn, lo) < 0)
{
! fprintf(stderr, "\nFailed to remove lo %u: ", lo);
! fprintf(stderr, "%s", PQerrorMessage(conn));
! if (PQtransactionStatus(conn) == PQTRANS_INERROR)
! {
! success = false;
! PQclear(res);
! break;
! }
}
+ else
+ deleted++;
}
else
deleted++;
!
! if (param->transaction_limit > 0 &&
! (deleted % param->transaction_limit) == 0)
{
! res2 = PQexec(conn, "commit");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to commit transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
! res2 = PQexec(conn, "begin");
! if (PQresultStatus(res2) != PGRES_COMMAND_OK)
! {
! fprintf(stderr, "Failed to start transaction:\n");
! fprintf(stderr, "%s", PQerrorMessage(conn));
! PQclear(res2);
! PQclear(res);
! PQfinish(conn);
! return -1;
! }
PQclear(res2);
}
}
+
+ PQclear(res);
}
/*
* That's all folks!
On Sun, Jul 7, 2013 at 9:30 AM, Josh Kupershmidt <schmiddy@gmail.com> wrote:
On Mon, Nov 12, 2012 at 5:14 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
vacuumlo is rather simpleminded about dealing with the list of LOs to be
removed - it just fetches them as a straight resultset. For one of my our
this resulted in an out of memory condition.Wow, they must have had a ton of LOs. With about 2M entries to pull
from vacuum_l, I observed unpatched vacuumlo using only about 45MB
RES. Still, the idea of using a cursor for the main loop seems like a
reasonable idea.The attached patch tries to
remedy that by using a cursor instead. If this is wanted I will add it to
the next commitfest. The actualy changes are very small - most of the patch
is indentation changes due to the introduction of an extra loop.I had some time to review this, some comments about the patch:
1. I see this new compiler warning:
vacuumlo.c: In function ‘vacuumlo’:
vacuumlo.c:306:5: warning: format ‘%lld’ expects argument of type
‘long long int’, but argument 4 has type ‘long int’ [-Wformat]2. It looks like the the patch causes all the intermediate result-sets
fetched from the cursor to be leaked, rather negating its stated
purpose ;-) The PQclear() call should be moved up into the main loop.
With this fixed, I confirmed that vacuumlo now consumes a negligible
amount of memory when chewing through millions of entries.3. A few extra trailing whitespaces were added.
4. The FETCH FORWARD count comes from transaction_limit. That seems
like a good-enough guesstimate, but maybe a comment could be added to
rationalize?Some suggested changes attached with v2 patch (all except #4).
I've committed this version of the patch, with a slight change to one
of the error messages.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers