Several more fixes for indexes-only mode:

* Fix error handling in repack_table_indexes() to keep track of which
   indexes were actually rebuilt successfully by pg_repack, so that only
   those indexes will be dropped. Previously, we would issue an error
   message telling the user to use DROP INDEX but then just drop the
   index anyway.
 * DROP INDEX command needs to quote the schema name in case it is
   not simple.
 * missing CLEARPGRES() in repack_all_indexes()
This commit is contained in:
Josh Kupershmidt 2013-06-29 09:41:27 -04:00
parent f0f36688f0
commit 1e8ac21336

View File

@ -281,7 +281,7 @@ main(int argc, char *argv[])
errmsg("ANALYZE is not performed after repacking indexes, -z (--no-analyze) has no effect"))); errmsg("ANALYZE is not performed after repacking indexes, -z (--no-analyze) has no effect")));
else if (jobs) else if (jobs)
ereport(WARNING, (errcode(EINVAL), ereport(WARNING, (errcode(EINVAL),
errmsg("option -j (--jobs) has no effect, repacking indexes doesnot use parallel jobs"))); errmsg("option -j (--jobs) has no effect, repacking indexes does not use parallel jobs")));
if (!repack_all_indexes(errbuf, sizeof(errbuf))) if (!repack_all_indexes(errbuf, sizeof(errbuf)))
ereport(ERROR, ereport(ERROR,
(errcode(ERROR), errmsg("%s", errbuf))); (errcode(ERROR), errmsg("%s", errbuf)));
@ -1615,10 +1615,10 @@ repack_table_indexes(PGresult *index_details)
PGresult *res = NULL; PGresult *res = NULL;
StringInfoData sql, sql_drop; StringInfoData sql, sql_drop;
char buffer[2][12]; char buffer[2][12];
char *create_idx; const char *create_idx, *schema_name, *table_name, *params[3];
const char *schema_name, *table_name, *params[3];
Oid table, index; Oid table, index;
int i, num, num_valid = -1; int i, num;
bool *repacked_indexes;
num = PQntuples(index_details); num = PQntuples(index_details);
table = getoid(index_details, 0, 3); table = getoid(index_details, 0, 3);
@ -1627,6 +1627,13 @@ repack_table_indexes(PGresult *index_details)
schema_name = getstr(index_details, 0, 5); schema_name = getstr(index_details, 0, 5);
table_name = getstr(index_details, 0, 4); table_name = getstr(index_details, 0, 4);
/* Keep track of which of the table's indexes we have successfully
* repacked, so that we may DROP only those indexes.
*/
if (!(repacked_indexes = calloc(num, sizeof(bool))))
ereport(ERROR, (errcode(ENOMEM),
errmsg("Unable to calloc repacked_indexes")));
/* Check if any concurrent pg_repack command is being run on the same /* Check if any concurrent pg_repack command is being run on the same
* table. * table.
*/ */
@ -1643,69 +1650,72 @@ repack_table_indexes(PGresult *index_details)
index = getoid(index_details, i, 1); index = getoid(index_details, i, 1);
params[0] = utoa(index, buffer[0]); params[0] = utoa(index, buffer[0]);
res = execute("SELECT repack.repack_indexdef($1, $2, $3, true)", 3, params); res = execute("SELECT repack.repack_indexdef($1, $2, $3, true)", 3,
params);
if (PQntuples(res) < 1) if (PQntuples(res) < 1)
{ {
elog(WARNING, elog(WARNING,
"unable to generate SQL to CREATE work index for %s.%s", "unable to generate SQL to CREATE work index for %s.%s",
schema_name, getstr(index_details, i, 0)); schema_name, getstr(index_details, i, 0));
num_valid = i; continue;
goto drop_idx;
} }
create_idx = getstr(res, 0, 0); create_idx = getstr(res, 0, 0);
CLEARPGRES(res); CLEARPGRES(res);
res = execute_elevel(create_idx, 0, NULL, DEBUG2);
res = execute_elevel(create_idx, 0, NULL, DEBUG2);
if (PQresultStatus(res) != PGRES_COMMAND_OK) if (PQresultStatus(res) != PGRES_COMMAND_OK)
{ {
ereport(WARNING, ereport(WARNING,
(errcode(E_PG_COMMAND), (errcode(E_PG_COMMAND),
errmsg("SQL failed with message- %s", errmsg("Error creating index: %s",
PQerrorMessage(connection)), PQerrorMessage(connection)),
errdetail("An invalid index may have been left behind" errdetail("An invalid index may have been left behind"
" by a pg_repack command on the table which" " by a previous pg_repack on the table"
" was interrupted and failed to clean up" " which was interrupted. Please use DROP "
" the temporary objects. Please use \"DROP INDEX %s.index_%u\"" "INDEX \"%s\".\"index_%u\""
" to remove this index and try again.", schema_name, index))); " to remove this index and try again.",
num_valid = i + 1; schema_name, index)));
ret = false;
goto drop_idx;
} }
else
repacked_indexes[i] = true;
CLEARPGRES(res); CLEARPGRES(res);
} }
else else
{ elog(WARNING, "skipping invalid index: %s.%s", schema_name,
if (num_valid == -1) getstr(index_details, i, 0));
num_valid = i;
elog(WARNING, "skipping invalid index: %s.%s", schema_name, getstr(index_details, i, 0));
}
} }
if (num_valid == -1)
num_valid = i;
/* take exclusive lock on table before calling repack_index_swap() */ /* take exclusive lock on table before calling repack_index_swap() */
initStringInfo(&sql); initStringInfo(&sql);
appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE", appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE",
table_name); table_name);
if (!(lock_exclusive(connection, params[1], sql.data, TRUE))) if (!(lock_exclusive(connection, params[1], sql.data, TRUE)))
{ {
elog(WARNING, "lock_exclusive() failed in connection for %s", elog(WARNING, "lock_exclusive() failed in connection for %s",
table_name); table_name);
goto drop_idx; goto drop_idx;
} }
for (i = 0; i < num_valid; i++) for (i = 0; i < num; i++)
{ {
index = getoid(index_details, i, 1); index = getoid(index_details, i, 1);
params[0] = utoa(index, buffer[0]); if (repacked_indexes[i])
pgut_command(connection, "SELECT repack.repack_index_swap($1)", 1, params); {
params[0] = utoa(index, buffer[0]);
pgut_command(connection, "SELECT repack.repack_index_swap($1)", 1,
params);
}
else
elog(INFO, "Skipping index swap for index_%u", index);
} }
pgut_command(connection, "COMMIT", 0, NULL); pgut_command(connection, "COMMIT", 0, NULL);
ret = true; ret = true;
drop_idx: drop_idx:
CLEARPGRES(res);
initStringInfo(&sql); initStringInfo(&sql);
initStringInfo(&sql_drop); initStringInfo(&sql_drop);
#if PG_VERSION_NUM < 90200 #if PG_VERSION_NUM < 90200
@ -1713,13 +1723,19 @@ drop_idx:
#else #else
appendStringInfoString(&sql, "DROP INDEX CONCURRENTLY IF EXISTS "); appendStringInfoString(&sql, "DROP INDEX CONCURRENTLY IF EXISTS ");
#endif #endif
appendStringInfo(&sql, "%s.", schema_name); appendStringInfo(&sql, "\"%s\".", schema_name);
for (i = 0; i < num_valid; i++) for (i = 0; i < num; i++)
{ {
initStringInfo(&sql_drop); index = getoid(index_details, i, 1);
appendStringInfo(&sql_drop, "%sindex_%u", sql.data, getoid(index_details, i, 1)); if (repacked_indexes[i])
command(sql_drop.data, 0, NULL); {
initStringInfo(&sql_drop);
appendStringInfo(&sql_drop, "%s\"index_%u\"", sql.data, index);
command(sql_drop.data, 0, NULL);
}
else
elog(INFO, "Skipping drop of index_%u", index);
} }
termStringInfo(&sql_drop); termStringInfo(&sql_drop);
termStringInfo(&sql); termStringInfo(&sql);
@ -1727,7 +1743,7 @@ drop_idx:
} }
/* /*
* Call repack_table_indexes for each of the table * Call repack_table_indexes for each of the tables
*/ */
static bool static bool
repack_all_indexes(char *errbuf, size_t errsize) repack_all_indexes(char *errbuf, size_t errsize)
@ -1774,8 +1790,7 @@ repack_all_indexes(char *errbuf, size_t errsize)
if (PQresultStatus(res) != PGRES_TUPLES_OK) if (PQresultStatus(res) != PGRES_TUPLES_OK)
{ {
elog(WARNING, "SQL failed with message- %s", elog(WARNING, "%s", PQerrorMessage(connection));
PQerrorMessage(connection));
continue; continue;
} }
@ -1798,6 +1813,8 @@ repack_all_indexes(char *errbuf, size_t errsize)
if (!repack_table_indexes(res)) if (!repack_table_indexes(res))
elog(WARNING, "repack failed for \"%s\"", cell->val); elog(WARNING, "repack failed for \"%s\"", cell->val);
CLEARPGRES(res);
} }
ret = true; ret = true;