Skip to content

Commit f0f3668

Browse files
schmiddyschmiddy
committed
A few fixes to the indexes-only building.
Improve locking behavior in indexes-only mode so we only need one ACCESS EXCLUSIVE lock per table instead of one per index. Support multiple --index options. Fix for schema-name parsing. Patch from Beena Emerson.
1 parent df53964 commit f0f3668

File tree

1 file changed

+136
-151
lines changed

1 file changed

+136
-151
lines changed

‎bin/pg_repack.c

Lines changed: 136 additions & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ static bool preliminary_checks(char *errbuf, size_t errsize);
180180
static void repack_all_databases(const char *order_by);
181181
static bool repack_one_database(const char *order_by, char *errbuf, size_t errsize);
182182
static void repack_one_table(const repack_table *table, const char *order_by);
183-
static bool repack_one_index(Oid table, const char *table_name, Oid index, const char *schema_name);
183+
static bool repack_table_indexes(PGresult *index_details);
184184
static bool repack_all_indexes(char *errbuf, size_t errsize);
185185
static void repack_cleanup(bool fatal, const repack_table *table);
186186
static bool rebuild_indexes(const repack_table *table);
@@ -208,7 +208,7 @@ static SimpleStringList table_list = {NULL, NULL};
208208
static char *orderby = NULL;
209209
static char *tablespace = NULL;
210210
static bool moveidx = false;
211-
static char *r_index = NULL;
211+
static SimpleStringList r_index = {NULL, NULL};
212212
static bool only_indexes = false;
213213
static int wait_timeout = 60; /* in seconds */
214214
static int jobs = 0; /* number of concurrent worker conns. */
@@ -229,7 +229,7 @@ static pgut_option options[] =
229229
{ 's', 'o', "order-by", &orderby },
230230
{ 's', 's', "tablespace", &tablespace },
231231
{ 'b', 'S', "moveidx", &moveidx },
232-
{ 's', 'i', "index", &r_index },
232+
{ 'l', 'i', "index", &r_index },
233233
{ 'b', 'x', "only-indexes", &only_indexes },
234234
{ 'i', 'T', "wait-timeout", &wait_timeout },
235235
{ 'B', 'Z', "no-analyze", &analyze },
@@ -254,12 +254,12 @@ main(int argc, char *argv[])
254254

255255
check_tablespace();
256256

257-
if (r_index || only_indexes)
257+
if (r_index.head || only_indexes)
258258
{
259-
if (r_index && table_list.head)
259+
if (r_index.head && table_list.head)
260260
ereport(ERROR, (errcode(EINVAL),
261261
errmsg("cannot specify --index (-i) and --table (-t)")));
262-
else if (r_index && only_indexes)
262+
else if (r_index.head && only_indexes)
263263
ereport(ERROR, (errcode(EINVAL),
264264
errmsg("cannot specify --index (-i) and --only-indexes (-x)")));
265265
else if (only_indexes && !table_list.head)
@@ -1606,213 +1606,198 @@ repack_cleanup(bool fatal, const repack_table *table)
16061606
}
16071607

16081608
/*
1609-
* repack one index
1609+
* Indexes of a table are repacked.
16101610
*/
16111611
static bool
1612-
repack_one_index(Oid table, const char *table_name, Oid index, const char *schema_name)
1612+
repack_table_indexes(PGresult *index_details)
16131613
{
16141614
bool ret = false;
16151615
PGresult *res = NULL;
1616-
StringInfoData sql, temp_index;
1616+
StringInfoData sql, sql_drop;
16171617
char buffer[2][12];
16181618
char *create_idx;
1619-
const char *params[3];
1619+
const char *schema_name, *table_name, *params[3];
1620+
Oid table, index;
1621+
int i, num, num_valid = -1;
16201622

1621-
params[0] = utoa(index, buffer[0]);
1623+
num = PQntuples(index_details);
1624+
table = getoid(index_details, 0, 3);
16221625
params[1] = utoa(table, buffer[1]);
16231626
params[2] = tablespace;
1624-
res = execute("SELECT repack.repack_indexdef($1, $2, $3, true)", 3, params);
1625-
if (PQntuples(res) < 1)
1626-
{
1627-
ereport(ERROR, (errcode(EINVAL),
1628-
errmsg("unable to generate SQL to CREATE new index")));
1629-
}
1630-
create_idx = getstr(res, 0, 0);
1631-
CLEARPGRES(res);
1632-
res = execute_elevel(create_idx, 0, NULL, DEBUG2);
1627+
schema_name = getstr(index_details, 0, 5);
1628+
table_name = getstr(index_details, 0, 4);
16331629

1634-
initStringInfo(&temp_index);
1635-
if (schema_name)
1636-
appendStringInfo(&temp_index, "%s.index_%u", schema_name, index);
1637-
else
1638-
appendStringInfo(&temp_index, "index_%u", index);
1630+
/* Check if any concurrent pg_repack command is being run on the same
1631+
* table.
1632+
*/
1633+
if (!advisory_lock(connection, params[1]))
1634+
ereport(ERROR, (errcode(EINVAL),
1635+
errmsg("Unable to obtain advisory lock on \"%s\"", table_name)));
16391636

1640-
if (PQresultStatus(res) != PGRES_COMMAND_OK)
1637+
for (i = 0; i < num; i++)
16411638
{
1642-
ereport(ERROR,
1643-
(errcode(E_PG_COMMAND),
1644-
errmsg("%s", PQerrorMessage(connection)),
1645-
errdetail("An invalid index may have been left behind "
1646-
" by a pg_repack command on the table which"
1647-
" was interrupted and failed to clean up"
1648-
" the temporary objects. Please use \"DROP INDEX %s\""
1649-
" to remove this index.", temp_index.data)));
1639+
char *isvalid = getstr(index_details, i, 2);
1640+
1641+
if (isvalid[0] == 't')
1642+
{
1643+
index = getoid(index_details, i, 1);
1644+
params[0] = utoa(index, buffer[0]);
1645+
1646+
res = execute("SELECT repack.repack_indexdef($1, $2, $3, true)", 3, params);
1647+
if (PQntuples(res) < 1)
1648+
{
1649+
elog(WARNING,
1650+
"unable to generate SQL to CREATE work index for %s.%s",
1651+
schema_name, getstr(index_details, i, 0));
1652+
num_valid = i;
1653+
goto drop_idx;
1654+
}
1655+
1656+
create_idx = getstr(res, 0, 0);
1657+
CLEARPGRES(res);
1658+
res = execute_elevel(create_idx, 0, NULL, DEBUG2);
1659+
1660+
if (PQresultStatus(res) != PGRES_COMMAND_OK)
1661+
{
1662+
ereport(WARNING,
1663+
(errcode(E_PG_COMMAND),
1664+
errmsg("SQL failed with message- %s",
1665+
PQerrorMessage(connection)),
1666+
errdetail("An invalid index may have been left behind"
1667+
" by a pg_repack command on the table which"
1668+
" was interrupted and failed to clean up"
1669+
" the temporary objects. Please use \"DROP INDEX %s.index_%u\""
1670+
" to remove this index and try again.", schema_name, index)));
1671+
num_valid = i + 1;
1672+
ret = false;
1673+
goto drop_idx;
1674+
}
1675+
CLEARPGRES(res);
1676+
}
1677+
else
1678+
{
1679+
if (num_valid == -1)
1680+
num_valid = i;
1681+
elog(WARNING, "skipping invalid index: %s.%s", schema_name, getstr(index_details, i, 0));
1682+
}
16501683
}
1651-
CLEARPGRES(res);
1684+
1685+
if (num_valid == -1)
1686+
num_valid = i;
16521687

16531688
/* take exclusive lock on table before calling repack_index_swap() */
16541689
initStringInfo(&sql);
1655-
if (schema_name)
1656-
appendStringInfo(&sql, "LOCK TABLE %s.%s IN ACCESS EXCLUSIVE MODE",
1657-
schema_name, table_name);
1658-
else
1659-
appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE",
1660-
table_name);
1690+
appendStringInfo(&sql, "LOCK TABLE %s IN ACCESS EXCLUSIVE MODE",
1691+
table_name);
16611692
if (!(lock_exclusive(connection, params[1], sql.data, TRUE)))
16621693
{
1663-
elog(WARNING, "lock_exclusive() failed in connection for %s",
1664-
table_name);
1694+
elog(WARNING, "lock_exclusive() failed in connection for %s",
1695+
table_name);
16651696
goto drop_idx;
16661697
}
1667-
pgut_command(connection, "SELECT repack.repack_index_swap($1)", 1, params);
1698+
1699+
for (i = 0; i < num_valid; i++)
1700+
{
1701+
index = getoid(index_details, i, 1);
1702+
params[0] = utoa(index, buffer[0]);
1703+
pgut_command(connection, "SELECT repack.repack_index_swap($1)", 1, params);
1704+
}
16681705
pgut_command(connection, "COMMIT", 0, NULL);
1669-
16701706
ret = true;
16711707

16721708
drop_idx:
16731709
initStringInfo(&sql);
1710+
initStringInfo(&sql_drop);
16741711
#if PG_VERSION_NUM < 90200
1675-
appendStringInfoString(&sql, "DROP INDEX ");
1712+
appendStringInfoString(&sql, "DROP INDEX IF EXISTS ");
16761713
#else
1677-
appendStringInfoString(&sql, "DROP INDEX CONCURRENTLY ");
1714+
appendStringInfoString(&sql, "DROP INDEX CONCURRENTLY IF EXISTS ");
16781715
#endif
1679-
appendStringInfo(&sql, "%s", temp_index.data);
1680-
command(sql.data, 0, NULL);
1716+
appendStringInfo(&sql, "%s.", schema_name);
16811717

1682-
CLEARPGRES(res);
1718+
for (i = 0; i < num_valid; i++)
1719+
{
1720+
initStringInfo(&sql_drop);
1721+
appendStringInfo(&sql_drop, "%sindex_%u", sql.data, getoid(index_details, i, 1));
1722+
command(sql_drop.data, 0, NULL);
1723+
}
1724+
termStringInfo(&sql_drop);
16831725
termStringInfo(&sql);
16841726
return ret;
16851727
}
16861728

16871729
/*
1688-
* Call repack_one_index for each of the indexes
1730+
* Call repack_table_indexes for each of the table
16891731
*/
16901732
static bool
16911733
repack_all_indexes(char *errbuf, size_t errsize)
16921734
{
1693-
bool ret = false;
1694-
PGresult *res = NULL;
1695-
int i;
1696-
int num;
1697-
StringInfoData sql;
1698-
char buffer[12];
1699-
const char *params[1];
1700-
const char *table_name = NULL;
1701-
const char *schema_name = NULL;
1702-
char *pos;
1735+
bool ret = false;
1736+
PGresult *res = NULL;
1737+
StringInfoData sql;
1738+
SimpleStringListCell *cell = NULL;
1739+
const char *params[1];
17031740

17041741
initStringInfo(&sql);
17051742
reconnect(ERROR);
17061743

1744+
assert(r_index.head || table_list.head);
1745+
17071746
if (!preliminary_checks(errbuf, errsize))
17081747
goto cleanup;
17091748

1710-
/* If only one index is specified, append the appropriate data to the sql
1711-
* and check if the index exists
1712-
*/
1713-
if (r_index)
1749+
if (r_index.head)
17141750
{
1715-
appendStringInfoString(&sql, "SELECT i.relname, idx.indexrelid, idx.indisvalid, tbl.oid, tbl.relname"
1716-
" FROM pg_class tbl JOIN pg_index idx ON tbl.oid = idx.indrelid"
1717-
" JOIN pg_class i ON i.oid = idx.indexrelid"
1718-
" WHERE idx.indexrelid = $1::regclass");
1719-
params[0] = r_index;
1720-
1721-
res = execute_elevel(sql.data, 1, params, DEBUG2);
1722-
1723-
if (PQresultStatus(res) != PGRES_TUPLES_OK)
1724-
{
1725-
snprintf(errbuf, errsize, "%s", PQerrorMessage(connection));
1726-
goto cleanup;
1727-
}
1728-
else
1729-
{
1730-
num = PQntuples(res);
1731-
if (num == 0)
1732-
{
1733-
ereport(ERROR,
1734-
(errcode(EINVAL),
1735-
errmsg("index \"%s\" does not exist.\n", r_index)));
1736-
}
1737-
}
1751+
appendStringInfoString(&sql,
1752+
"SELECT i.relname, idx.indexrelid, idx.indisvalid, idx.indrelid, idx.indrelid::regclass, n.nspname"
1753+
" FROM pg_index idx JOIN pg_class i ON i.oid = idx.indexrelid"
1754+
" JOIN pg_namespace n ON n.oid = i.relnamespace"
1755+
" WHERE idx.indexrelid = $1::regclass ORDER BY indisvalid DESC");
17381756

1739-
// separate schema name and index name. FIXME: kludge
1740-
pos = strchr(params[0], '.');
1741-
if (pos)
1742-
{
1743-
pos[0] = '\0';
1744-
schema_name = params[0];
1745-
r_index = pos + 1;
1746-
}
1747-
table_name = getstr(res, 0, 4);
1757+
cell = r_index.head;
17481758
}
1749-
/* To repack all indexes, append appropriate data to the sql and run the query */
1750-
else {
1751-
params[0] = table_list.head->val;
1752-
1753-
appendStringInfoString(&sql, "SELECT i.relname, idx.indexrelid, idx.indisvalid, idx.indrelid"
1759+
else if(table_list.head)
1760+
{
1761+
appendStringInfoString(&sql,
1762+
"SELECT i.relname, idx.indexrelid, idx.indisvalid, idx.indrelid, $1::text, n.nspname"
17541763
" FROM pg_index idx JOIN pg_class i ON i.oid = idx.indexrelid"
1755-
" WHERE idx.indrelid = $1::regclass");
1764+
" JOIN pg_namespace n ON n.oid = i.relnamespace"
1765+
" WHERE idx.indrelid = $1::regclass ORDER BY indisvalid DESC");
17561766

1767+
cell = table_list.head;
1768+
}
1769+
1770+
for (; cell; cell = cell->next)
1771+
{
1772+
params[0] = cell->val;
17571773
res = execute_elevel(sql.data, 1, params, DEBUG2);
17581774

17591775
if (PQresultStatus(res) != PGRES_TUPLES_OK)
17601776
{
1761-
snprintf(errbuf, errsize, "%s", PQerrorMessage(connection));
1762-
goto cleanup;
1777+
elog(WARNING, "SQL failed with message- %s",
1778+
PQerrorMessage(connection));
1779+
continue;
17631780
}
1764-
else
1781+
1782+
if (PQntuples(res) == 0)
17651783
{
1766-
num = PQntuples(res);
1767-
if (num == 0)
1768-
{
1784+
if(table_list.head)
17691785
elog(WARNING, "\"%s\" does not have any indexes",
1770-
table_list.head->val);
1771-
ret = true;
1772-
goto cleanup;
1773-
}
1774-
}
1786+
cell->val);
1787+
else if(r_index.head)
1788+
elog(WARNING, "\"%s\" is not a valid index",
1789+
cell->val);
17751790

1776-
// separate schema name and table name. FIXME: kludge
1777-
pos = strchr(params[0], '.');
1778-
if (pos)
1779-
{
1780-
pos[0] = '\0';
1781-
schema_name = params[0];
1782-
table_name = pos + 1;
1791+
continue;
17831792
}
1784-
else
1785-
table_name = params[0];
1786-
}
1787-
1788-
/* Check if any concurrent pg_repack command is being run on the same
1789-
* table.
1790-
*/
1791-
if (!advisory_lock(connection, utoa(getoid(res, 0, 3), buffer))) {
1792-
snprintf(errbuf, errsize, "Unable to obtain advisory lock on \"%s\"",
1793-
table_name);
1794-
goto cleanup;
1795-
}
1796-
for (i = 0; i < num; i++)
1797-
{
1798-
char *isvalid = getstr(res, i, 2);
1799-
if (isvalid[0] == 't')
1800-
{
1801-
if (schema_name)
1802-
elog(INFO, "repacking index \"%s.%s\"", schema_name, getstr(res, i, 0));
1803-
else
1804-
elog(INFO, "repacking index \"%s\"", getstr(res, i, 0));
18051793

1806-
if (!(repack_one_index(getoid(res, i, 3), table_name, getoid(res, i, 1), schema_name))) {
1807-
/* FIXME: fill in errbuf here */
1808-
goto cleanup;
1809-
}
1810-
}
1794+
if(table_list.head)
1795+
elog(INFO, "repacking indexes of \"%s\"", cell->val);
18111796
else
1812-
if (schema_name)
1813-
elog(WARNING, "skipping invalid index: %s.%s", schema_name, getstr(res, i, 0));
1814-
else
1815-
elog(WARNING, "skipping invalid index: %s", getstr(res, i, 0));
1797+
elog(INFO, "repacking \"%s\"", cell->val);
1798+
1799+
if (!repack_table_indexes(res))
1800+
elog(WARNING, "repack failed for \"%s\"", cell->val);
18161801
}
18171802
ret = true;
18181803

0 commit comments

Comments
 (0)