A few cosmetic fixes to pg_repack.c:

* fix old reference to --indexes-only
 * trailing whitespace and indentation cleanup to match rest of code
 * XXX note about improving advisory locking
This commit is contained in:
Josh Kupershmidt 2013-06-22 06:51:41 -04:00
parent b804c150ac
commit 62b5e4fb11

View File

@ -257,7 +257,7 @@ main(int argc, char *argv[])
errmsg("cannot specify --index (-i) and --table (-t)"))); errmsg("cannot specify --index (-i) and --table (-t)")));
else if (r_index && only_indexes) else if (r_index && only_indexes)
ereport(ERROR, (errcode(EINVAL), ereport(ERROR, (errcode(EINVAL),
errmsg("cannot specify --index (-i) and --indexes_only (-x)"))); errmsg("cannot specify --index (-i) and --only-index (-x)")));
else if (only_indexes && !table_list.head) else if (only_indexes && !table_list.head)
ereport(ERROR, (errcode(EINVAL), ereport(ERROR, (errcode(EINVAL),
errmsg("cannot repack all indexes of database, specify the table with -t option"))); errmsg("cannot repack all indexes of database, specify the table with -t option")));
@ -969,7 +969,7 @@ repack_one_table(const repack_table *table, const char *orderby)
/* Keep track of whether we have gotten through setup to install /* Keep track of whether we have gotten through setup to install
* the z_repack_trigger, log table, etc. ourselves. We don't want to * the z_repack_trigger, log table, etc. ourselves. We don't want to
* go through repack_cleanup() if we didnt' actually set up the * go through repack_cleanup() if we didn't actually set up the
* trigger ourselves, lest we be cleaning up another pg_repack's mess, * trigger ourselves, lest we be cleaning up another pg_repack's mess,
* or worse, interfering with a still-running pg_repack. * or worse, interfering with a still-running pg_repack.
*/ */
@ -1012,7 +1012,7 @@ repack_one_table(const repack_table *table, const char *orderby)
} }
/* /*
* Check z_repack_trigger is the trigger executed at last so that * Check z_repack_trigger is the trigger executed last so that
* other before triggers cannot modify triggered tuples. * other before triggers cannot modify triggered tuples.
*/ */
params[0] = utoa(table->target_oid, buffer); params[0] = utoa(table->target_oid, buffer);
@ -1578,7 +1578,8 @@ repack_cleanup(bool fatal, const repack_table *table)
* repack one index * repack one index
*/ */
static bool static bool
repack_one_index(Oid table, const char *table_name, Oid index, const char *schema_name){ repack_one_index(Oid table, const char *table_name, Oid index, const char *schema_name)
{
bool ret = false; bool ret = false;
PGresult *res = NULL; PGresult *res = NULL;
StringInfoData sql, temp_index; StringInfoData sql, temp_index;
@ -1611,11 +1612,11 @@ repack_one_index(Oid table, const char *table_name, Oid index, const char *schem
ereport(ERROR, ereport(ERROR,
(errcode(E_PG_COMMAND), (errcode(E_PG_COMMAND),
errmsg("%s", PQerrorMessage(connection)), errmsg("%s", PQerrorMessage(connection)),
errdetail("The temporary index may be left behind " errdetail("An invalid index may have been left behind "
" by a pg_repack command on the table which" " by a pg_repack command on the table which"
" was interrupted and failed to clean up" " was interrupted and failed to clean up"
" the temporary objects. Please use the \"DROP INDEX %s\"" " the temporary objects. Please use \"DROP INDEX %s\""
" to remove the temporary index.", temp_index.data))); " to remove this index.", temp_index.data)));
goto cleanup; goto cleanup;
} }
CLEARPGRES(res); CLEARPGRES(res);
@ -1623,9 +1624,11 @@ repack_one_index(Oid table, const char *table_name, Oid index, const char *schem
/* take exclusive lock on table before calling repack_index_swap() */ /* take exclusive lock on table before calling repack_index_swap() */
initStringInfo(&sql); initStringInfo(&sql);
if (schema_name) if (schema_name)
appendStringInfo(&sql, "LOCK TABLE %s.%s IN ACCESS EXCLUSIVE MODE", schema_name, table_name); appendStringInfo(&sql, "LOCK TABLE %s.%s IN ACCESS EXCLUSIVE MODE",
schema_name, table_name);
else else
appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE", table_name); appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE",
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",
@ -1645,6 +1648,7 @@ drop_idx:
appendStringInfo(&sql, "%s", temp_index.data); appendStringInfo(&sql, "%s", temp_index.data);
command(sql.data, 0, NULL); command(sql.data, 0, NULL);
ret = true; ret = true;
cleanup: cleanup:
CLEARPGRES(res); CLEARPGRES(res);
termStringInfo(&sql); termStringInfo(&sql);
@ -1672,7 +1676,9 @@ repack_all_indexes(char *errbuf, size_t errsize){
if (!preliminary_checks(errbuf, errsize)) if (!preliminary_checks(errbuf, errsize))
goto cleanup; goto cleanup;
/* If only one index is specified, append the appropriate data to the sql and check if the index exists */ /* If only one index is specified, append the appropriate data to the sql
* and check if the index exists
*/
if (r_index) if (r_index)
{ {
appendStringInfoString(&sql, "SELECT i.relname, idx.indexrelid, idx.indisvalid, tbl.oid, tbl.relname" appendStringInfoString(&sql, "SELECT i.relname, idx.indexrelid, idx.indisvalid, tbl.oid, tbl.relname"
@ -1730,7 +1736,7 @@ repack_all_indexes(char *errbuf, size_t errsize){
num = PQntuples(res); num = PQntuples(res);
if (num == 0) if (num == 0)
{ {
elog(WARNING, "\"%s\" doesnot have any indexes", table_list.head->val); elog(WARNING, "\"%s\" does not have any indexes", table_list.head->val);
ret = true; ret = true;
goto cleanup; goto cleanup;
} }
@ -1748,6 +1754,14 @@ repack_all_indexes(char *errbuf, size_t errsize){
table_name = params[0]; table_name = params[0];
} }
/* XXX: Make sure that repack_one_table() also obtains an advisory
* lock on the table, so that we can't have a table-wide repack running
* along with an indexes-only repack. Also, since advisory locks are
* 8 bytes wide and OIDs are only 4 bytes, consider using our own prefix
* rather than just the table OID, to avoid inadvertent conflict with
* other applications using advisory locks.
*/
/* Check if any concurrent pg_repack command is being run on the same table */ /* Check if any concurrent pg_repack command is being run on the same table */
initStringInfo(&sql); initStringInfo(&sql);
appendStringInfo(&sql, "SELECT pg_try_advisory_lock(%u)", getoid(res, 0, 3)); appendStringInfo(&sql, "SELECT pg_try_advisory_lock(%u)", getoid(res, 0, 3));