public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v27 1/5] Remove pg_collation.collversion. 3+ messages / 2 participants [nested] [flat]
* [PATCH v27 1/5] Remove pg_collation.collversion. @ 2019-05-28 18:15 Thomas Munro <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Thomas Munro @ 2019-05-28 18:15 UTC (permalink / raw) While this allowed the system to detect changes in collation versions, it didn't help us track what had to be done about it. A later patch will add version tracking for individual database objects that depend on collations. Author: Thomas Munro <[email protected]> Reviewed-by: Julien Rouhaud <[email protected]> Reviewed-by: Peter Eisentraut <[email protected]> Reviewed-by: Michael Paquier <[email protected]> Discussion: https://postgr.es/m/CAEepm%3D0uEQCpfq_%2BLYFBdArCe4Ot98t1aR4eYiYTe%3DyavQygiQ%40mail.gmail.com --- doc/src/sgml/catalogs.sgml | 11 --- doc/src/sgml/func.sgml | 6 +- doc/src/sgml/ref/alter_collation.sgml | 63 ------------- doc/src/sgml/ref/create_collation.sgml | 21 ----- src/backend/catalog/pg_collation.c | 5 -- src/backend/commands/collationcmds.c | 88 ------------------- src/backend/nodes/copyfuncs.c | 13 --- src/backend/nodes/equalfuncs.c | 11 --- src/backend/parser/gram.y | 18 +--- src/backend/tcop/utility.c | 12 --- src/backend/utils/adt/pg_locale.c | 37 -------- src/bin/pg_dump/pg_dump.c | 24 +---- src/include/catalog/pg_collation.dat | 7 +- src/include/catalog/pg_collation.h | 5 -- src/include/catalog/toasting.h | 1 - src/include/commands/collationcmds.h | 1 - src/include/nodes/parsenodes.h | 11 --- .../regress/expected/collate.icu.utf8.out | 3 - .../regress/expected/collate.linux.utf8.out | 3 - src/test/regress/sql/collate.icu.utf8.sql | 5 -- src/test/regress/sql/collate.linux.utf8.sql | 5 -- 21 files changed, 7 insertions(+), 343 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 26fda20d19..a28ae89e13 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -2353,17 +2353,6 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l <symbol>LC_CTYPE</symbol> for this collation object </para></entry> </row> - - <row> - <entry role="catalog_table_entry"><para role="column_definition"> - <structfield>collversion</structfield> <type>text</type> - </para> - <para> - Provider-specific version of the collation. This is recorded when the - collation is created and then checked when it is used, to detect - changes in the collation definition that could lead to data corruption. - </para></entry> - </row> </tbody> </tgroup> </table> diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index f766c1bc67..8432e790f2 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -25438,11 +25438,7 @@ postgres=# SELECT * FROM pg_walfile_name_offset(pg_stop_backup()); </para> <para> Returns the actual version of the collation object as it is currently - installed in the operating system. If this is different from the - value in - <structname>pg_collation</structname>.<structfield>collversion</structfield>, - then objects depending on the collation might need to be rebuilt. See - also <xref linkend="sql-altercollation"/>. + installed in the operating system. </para></entry> </row> diff --git a/doc/src/sgml/ref/alter_collation.sgml b/doc/src/sgml/ref/alter_collation.sgml index bee6f0dd3c..c985b0de56 100644 --- a/doc/src/sgml/ref/alter_collation.sgml +++ b/doc/src/sgml/ref/alter_collation.sgml @@ -21,8 +21,6 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> -ALTER COLLATION <replaceable>name</replaceable> REFRESH VERSION - ALTER COLLATION <replaceable>name</replaceable> RENAME TO <replaceable>new_name</replaceable> ALTER COLLATION <replaceable>name</replaceable> OWNER TO { <replaceable>new_owner</replaceable> | CURRENT_USER | SESSION_USER } ALTER COLLATION <replaceable>name</replaceable> SET SCHEMA <replaceable>new_schema</replaceable> @@ -88,70 +86,9 @@ ALTER COLLATION <replaceable>name</replaceable> SET SCHEMA <replaceable>new_sche </listitem> </varlistentry> - <varlistentry> - <term><literal>REFRESH VERSION</literal></term> - <listitem> - <para> - Update the collation's version. - See <xref linkend="sql-altercollation-notes"/> below. - </para> - </listitem> - </varlistentry> </variablelist> </refsect1> - <refsect1 id="sql-altercollation-notes" xreflabel="Notes"> - <title>Notes</title> - - <para> - When using collations provided by the ICU library, the ICU-specific version - of the collator is recorded in the system catalog when the collation object - is created. When the collation is used, the current version is - checked against the recorded version, and a warning is issued when there is - a mismatch, for example: -<screen> -WARNING: collation "xx-x-icu" has version mismatch -DETAIL: The collation in the database was created using version 1.2.3.4, but the operating system provides version 2.3.4.5. -HINT: Rebuild all objects affected by this collation and run ALTER COLLATION pg_catalog."xx-x-icu" REFRESH VERSION, or build PostgreSQL with the right library version. -</screen> - A change in collation definitions can lead to corrupt indexes and other - problems because the database system relies on stored objects having a - certain sort order. Generally, this should be avoided, but it can happen - in legitimate circumstances, such as when - using <command>pg_upgrade</command> to upgrade to server binaries linked - with a newer version of ICU. When this happens, all objects depending on - the collation should be rebuilt, for example, - using <command>REINDEX</command>. When that is done, the collation version - can be refreshed using the command <literal>ALTER COLLATION ... REFRESH - VERSION</literal>. This will update the system catalog to record the - current collator version and will make the warning go away. Note that this - does not actually check whether all affected objects have been rebuilt - correctly. - </para> - <para> - When using collations provided by <literal>libc</literal> and - <productname>PostgreSQL</productname> was built with the GNU C library, the - C library's version is used as a collation version. Since collation - definitions typically change only with GNU C library releases, this provides - some defense against corruption, but it is not completely reliable. - </para> - <para> - Currently, there is no version tracking for the database default collation. - </para> - - <para> - The following query can be used to identify all collations in the current - database that need to be refreshed and the objects that depend on them: -<programlisting><![CDATA[ -SELECT pg_describe_object(refclassid, refobjid, refobjsubid) AS "Collation", - pg_describe_object(classid, objid, objsubid) AS "Object" - FROM pg_depend d JOIN pg_collation c - ON refclassid = 'pg_collation'::regclass AND refobjid = c.oid - WHERE c.collversion <> pg_collation_actual_version(c.oid) - ORDER BY 1, 2; -]]></programlisting></para> - </refsect1> - <refsect1> <title>Examples</title> diff --git a/doc/src/sgml/ref/create_collation.sgml b/doc/src/sgml/ref/create_collation.sgml index 58f5f0cd63..b97842071f 100644 --- a/doc/src/sgml/ref/create_collation.sgml +++ b/doc/src/sgml/ref/create_collation.sgml @@ -27,7 +27,6 @@ CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> ( [ LC_CTYPE = <replaceable>lc_ctype</replaceable>, ] [ PROVIDER = <replaceable>provider</replaceable>, ] [ DETERMINISTIC = <replaceable>boolean</replaceable>, ] - [ VERSION = <replaceable>version</replaceable> ] ) CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> FROM <replaceable>existing_collation</replaceable> </synopsis> @@ -149,26 +148,6 @@ CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> FROM <replace </listitem> </varlistentry> - <varlistentry> - <term><replaceable>version</replaceable></term> - - <listitem> - <para> - Specifies the version string to store with the collation. Normally, - this should be omitted, which will cause the version to be computed - from the actual version of the collation as provided by the operating - system. This option is intended to be used - by <command>pg_upgrade</command> for copying the version from an - existing installation. - </para> - - <para> - See also <xref linkend="sql-altercollation"/> for how to handle - collation version mismatches. - </para> - </listitem> - </varlistentry> - <varlistentry> <term><replaceable>existing_collation</replaceable></term> diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c index 5fdf1acb7e..3c84378d02 100644 --- a/src/backend/catalog/pg_collation.c +++ b/src/backend/catalog/pg_collation.c @@ -49,7 +49,6 @@ CollationCreate(const char *collname, Oid collnamespace, bool collisdeterministic, int32 collencoding, const char *collcollate, const char *collctype, - const char *collversion, bool if_not_exists, bool quiet) { @@ -167,10 +166,6 @@ CollationCreate(const char *collname, Oid collnamespace, values[Anum_pg_collation_collcollate - 1] = NameGetDatum(&name_collate); namestrcpy(&name_ctype, collctype); values[Anum_pg_collation_collctype - 1] = NameGetDatum(&name_ctype); - if (collversion) - values[Anum_pg_collation_collversion - 1] = CStringGetTextDatum(collversion); - else - nulls[Anum_pg_collation_collversion - 1] = true; tup = heap_form_tuple(tupDesc, values, nulls); diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index 9f6582c530..5ad8886e60 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -61,14 +61,12 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e DefElem *lcctypeEl = NULL; DefElem *providerEl = NULL; DefElem *deterministicEl = NULL; - DefElem *versionEl = NULL; char *collcollate = NULL; char *collctype = NULL; char *collproviderstr = NULL; bool collisdeterministic = true; int collencoding = 0; char collprovider = 0; - char *collversion = NULL; Oid newoid; ObjectAddress address; @@ -96,8 +94,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e defelp = &providerEl; else if (strcmp(defel->defname, "deterministic") == 0) defelp = &deterministicEl; - else if (strcmp(defel->defname, "version") == 0) - defelp = &versionEl; else { ereport(ERROR, @@ -166,9 +162,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e if (deterministicEl) collisdeterministic = defGetBoolean(deterministicEl); - if (versionEl) - collversion = defGetString(versionEl); - if (collproviderstr) { if (pg_strcasecmp(collproviderstr, "icu") == 0) @@ -215,9 +208,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e } } - if (!collversion) - collversion = get_collation_actual_version(collprovider, collcollate); - newoid = CollationCreate(collName, collNamespace, GetUserId(), @@ -226,7 +216,6 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e collencoding, collcollate, collctype, - collversion, if_not_exists, false); /* not quiet */ @@ -277,80 +266,6 @@ IsThereCollationInNamespace(const char *collname, Oid nspOid) collname, get_namespace_name(nspOid)))); } -/* - * ALTER COLLATION - */ -ObjectAddress -AlterCollation(AlterCollationStmt *stmt) -{ - Relation rel; - Oid collOid; - HeapTuple tup; - Form_pg_collation collForm; - Datum collversion; - bool isnull; - char *oldversion; - char *newversion; - ObjectAddress address; - - rel = table_open(CollationRelationId, RowExclusiveLock); - collOid = get_collation_oid(stmt->collname, false); - - if (!pg_collation_ownercheck(collOid, GetUserId())) - aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_COLLATION, - NameListToString(stmt->collname)); - - tup = SearchSysCacheCopy1(COLLOID, ObjectIdGetDatum(collOid)); - if (!HeapTupleIsValid(tup)) - elog(ERROR, "cache lookup failed for collation %u", collOid); - - collForm = (Form_pg_collation) GETSTRUCT(tup); - collversion = SysCacheGetAttr(COLLOID, tup, Anum_pg_collation_collversion, - &isnull); - oldversion = isnull ? NULL : TextDatumGetCString(collversion); - - newversion = get_collation_actual_version(collForm->collprovider, NameStr(collForm->collcollate)); - - /* cannot change from NULL to non-NULL or vice versa */ - if ((!oldversion && newversion) || (oldversion && !newversion)) - elog(ERROR, "invalid collation version change"); - else if (oldversion && newversion && strcmp(newversion, oldversion) != 0) - { - bool nulls[Natts_pg_collation]; - bool replaces[Natts_pg_collation]; - Datum values[Natts_pg_collation]; - - ereport(NOTICE, - (errmsg("changing version from %s to %s", - oldversion, newversion))); - - memset(values, 0, sizeof(values)); - memset(nulls, false, sizeof(nulls)); - memset(replaces, false, sizeof(replaces)); - - values[Anum_pg_collation_collversion - 1] = CStringGetTextDatum(newversion); - replaces[Anum_pg_collation_collversion - 1] = true; - - tup = heap_modify_tuple(tup, RelationGetDescr(rel), - values, nulls, replaces); - } - else - ereport(NOTICE, - (errmsg("version has not changed"))); - - CatalogTupleUpdate(rel, &tup->t_self, tup); - - InvokeObjectPostAlterHook(CollationRelationId, collOid, 0); - - ObjectAddressSet(address, CollationRelationId, collOid); - - heap_freetuple(tup); - table_close(rel, NoLock); - - return address; -} - - Datum pg_collation_actual_version(PG_FUNCTION_ARGS) { @@ -608,7 +523,6 @@ pg_import_system_collations(PG_FUNCTION_ARGS) collid = CollationCreate(localebuf, nspid, GetUserId(), COLLPROVIDER_LIBC, true, enc, localebuf, localebuf, - get_collation_actual_version(COLLPROVIDER_LIBC, localebuf), true, true); if (OidIsValid(collid)) { @@ -669,7 +583,6 @@ pg_import_system_collations(PG_FUNCTION_ARGS) collid = CollationCreate(alias, nspid, GetUserId(), COLLPROVIDER_LIBC, true, enc, locale, locale, - get_collation_actual_version(COLLPROVIDER_LIBC, locale), true, true); if (OidIsValid(collid)) { @@ -731,7 +644,6 @@ pg_import_system_collations(PG_FUNCTION_ARGS) nspid, GetUserId(), COLLPROVIDER_ICU, true, -1, collcollate, collcollate, - get_collation_actual_version(COLLPROVIDER_ICU, collcollate), true, true); if (OidIsValid(collid)) { diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 89c409de66..2218f6e3db 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3226,16 +3226,6 @@ _copyAlterTableCmd(const AlterTableCmd *from) return newnode; } -static AlterCollationStmt * -_copyAlterCollationStmt(const AlterCollationStmt *from) -{ - AlterCollationStmt *newnode = makeNode(AlterCollationStmt); - - COPY_NODE_FIELD(collname); - - return newnode; -} - static AlterDomainStmt * _copyAlterDomainStmt(const AlterDomainStmt *from) { @@ -5232,9 +5222,6 @@ copyObjectImpl(const void *from) case T_AlterTableCmd: retval = _copyAlterTableCmd(from); break; - case T_AlterCollationStmt: - retval = _copyAlterCollationStmt(from); - break; case T_AlterDomainStmt: retval = _copyAlterDomainStmt(from); break; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index e3f33c40be..40eb879176 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1107,14 +1107,6 @@ _equalAlterTableCmd(const AlterTableCmd *a, const AlterTableCmd *b) return true; } -static bool -_equalAlterCollationStmt(const AlterCollationStmt *a, const AlterCollationStmt *b) -{ - COMPARE_NODE_FIELD(collname); - - return true; -} - static bool _equalAlterDomainStmt(const AlterDomainStmt *a, const AlterDomainStmt *b) { @@ -3284,9 +3276,6 @@ equal(const void *a, const void *b) case T_AlterTableCmd: retval = _equalAlterTableCmd(a, b); break; - case T_AlterCollationStmt: - retval = _equalAlterCollationStmt(a, b); - break; case T_AlterDomainStmt: retval = _equalAlterDomainStmt(a, b); break; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index dbb47d4982..e432049f8e 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -254,7 +254,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); } %type <node> stmt schema_stmt - AlterEventTrigStmt AlterCollationStmt + AlterEventTrigStmt AlterDatabaseStmt AlterDatabaseSetStmt AlterDomainStmt AlterEnumStmt AlterFdwStmt AlterForeignServerStmt AlterGroupStmt AlterObjectDependsStmt AlterObjectSchemaStmt AlterOwnerStmt @@ -838,7 +838,6 @@ stmtmulti: stmtmulti ';' stmt stmt : AlterEventTrigStmt - | AlterCollationStmt | AlterDatabaseStmt | AlterDatabaseSetStmt | AlterDefaultPrivilegesStmt @@ -10137,21 +10136,6 @@ drop_option: } ; -/***************************************************************************** - * - * ALTER COLLATION - * - *****************************************************************************/ - -AlterCollationStmt: ALTER COLLATION any_name REFRESH VERSION_P - { - AlterCollationStmt *n = makeNode(AlterCollationStmt); - n->collname = $3; - $$ = (Node *)n; - } - ; - - /***************************************************************************** * * ALTER SYSTEM diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 9b0c376c8c..fb8a964cc4 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1795,10 +1795,6 @@ ProcessUtilitySlow(ParseState *pstate, address = AlterStatistics((AlterStatsStmt *) parsetree); break; - case T_AlterCollationStmt: - address = AlterCollation((AlterCollationStmt *) parsetree); - break; - default: elog(ERROR, "unrecognized node type: %d", (int) nodeTag(parsetree)); @@ -2946,10 +2942,6 @@ CreateCommandTag(Node *parsetree) tag = CMDTAG_DROP_SUBSCRIPTION; break; - case T_AlterCollationStmt: - tag = CMDTAG_ALTER_COLLATION; - break; - case T_PrepareStmt: tag = CMDTAG_PREPARE; break; @@ -3562,10 +3554,6 @@ GetCommandLogLevel(Node *parsetree) lev = LOGSTMT_DDL; break; - case T_AlterCollationStmt: - lev = LOGSTMT_DDL; - break; - /* already-planned queries */ case T_PlannedStmt: { diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index 07299dbc09..514e0fa0af 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -1513,8 +1513,6 @@ pg_newlocale_from_collation(Oid collid) const char *collctype pg_attribute_unused(); struct pg_locale_struct result; pg_locale_t resultp; - Datum collversion; - bool isnull; tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collid)); if (!HeapTupleIsValid(tp)) @@ -1616,41 +1614,6 @@ pg_newlocale_from_collation(Oid collid) #endif /* not USE_ICU */ } - collversion = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion, - &isnull); - if (!isnull) - { - char *actual_versionstr; - char *collversionstr; - - actual_versionstr = get_collation_actual_version(collform->collprovider, collcollate); - if (!actual_versionstr) - { - /* - * This could happen when specifying a version in CREATE - * COLLATION for a libc locale, or manually creating a mess in - * the catalogs. - */ - ereport(ERROR, - (errmsg("collation \"%s\" has no actual version, but a version was specified", - NameStr(collform->collname)))); - } - collversionstr = TextDatumGetCString(collversion); - - if (strcmp(actual_versionstr, collversionstr) != 0) - ereport(WARNING, - (errmsg("collation \"%s\" has version mismatch", - NameStr(collform->collname)), - errdetail("The collation in the database was created using version %s, " - "but the operating system provides version %s.", - collversionstr, actual_versionstr), - errhint("Rebuild all objects affected by this collation and run " - "ALTER COLLATION %s REFRESH VERSION, " - "or build PostgreSQL with the right library version.", - quote_qualified_identifier(get_namespace_name(collform->collnamespace), - NameStr(collform->collname))))); - } - ReleaseSysCache(tp); /* We'll keep the pg_locale_t structures in TopMemoryContext */ diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 9c8436dde6..af8d38488e 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -13548,12 +13548,10 @@ dumpCollation(Archive *fout, CollInfo *collinfo) if (fout->remoteVersion >= 100000) appendPQExpBufferStr(query, - "collprovider, " - "collversion, "); + "collprovider, "); else appendPQExpBufferStr(query, - "'c' AS collprovider, " - "NULL AS collversion, "); + "'c' AS collprovider, "); if (fout->remoteVersion >= 120000) appendPQExpBufferStr(query, @@ -13614,24 +13612,6 @@ dumpCollation(Archive *fout, CollInfo *collinfo) appendStringLiteralAH(q, collctype, fout); } - /* - * For binary upgrade, carry over the collation version. For normal - * dump/restore, omit the version, so that it is computed upon restore. - */ - if (dopt->binary_upgrade) - { - int i_collversion; - - i_collversion = PQfnumber(res, "collversion"); - if (!PQgetisnull(res, 0, i_collversion)) - { - appendPQExpBufferStr(q, ", version = "); - appendStringLiteralAH(q, - PQgetvalue(res, 0, i_collversion), - fout); - } - } - appendPQExpBufferStr(q, ");\n"); if (dopt->binary_upgrade) diff --git a/src/include/catalog/pg_collation.dat b/src/include/catalog/pg_collation.dat index ba1b3e201b..45301ccdd7 100644 --- a/src/include/catalog/pg_collation.dat +++ b/src/include/catalog/pg_collation.dat @@ -15,17 +15,16 @@ { oid => '100', oid_symbol => 'DEFAULT_COLLATION_OID', descr => 'database\'s default collation', collname => 'default', collnamespace => 'PGNSP', collowner => 'PGUID', - collprovider => 'd', collencoding => '-1', collcollate => '', collctype => '', - collversion => '_null_' }, + collprovider => 'd', collencoding => '-1', collcollate => '', collctype => '' }, { oid => '950', oid_symbol => 'C_COLLATION_OID', descr => 'standard C collation', collname => 'C', collnamespace => 'PGNSP', collowner => 'PGUID', collprovider => 'c', collencoding => '-1', collcollate => 'C', - collctype => 'C', collversion => '_null_' }, + collctype => 'C' }, { oid => '951', oid_symbol => 'POSIX_COLLATION_OID', descr => 'standard POSIX collation', collname => 'POSIX', collnamespace => 'PGNSP', collowner => 'PGUID', collprovider => 'c', collencoding => '-1', collcollate => 'POSIX', - collctype => 'POSIX', collversion => '_null_' }, + collctype => 'POSIX' }, ] diff --git a/src/include/catalog/pg_collation.h b/src/include/catalog/pg_collation.h index 27618b324d..e7e958b808 100644 --- a/src/include/catalog/pg_collation.h +++ b/src/include/catalog/pg_collation.h @@ -37,10 +37,6 @@ CATALOG(pg_collation,3456,CollationRelationId) int32 collencoding; /* encoding for this collation; -1 = "all" */ NameData collcollate; /* LC_COLLATE setting */ NameData collctype; /* LC_CTYPE setting */ -#ifdef CATALOG_VARLEN /* variable-length fields start here */ - text collversion; /* provider-dependent version of collation - * data */ -#endif } FormData_pg_collation; /* ---------------- @@ -65,7 +61,6 @@ extern Oid CollationCreate(const char *collname, Oid collnamespace, bool collisdeterministic, int32 collencoding, const char *collcollate, const char *collctype, - const char *collversion, bool if_not_exists, bool quiet); diff --git a/src/include/catalog/toasting.h b/src/include/catalog/toasting.h index 51491c4513..8f131893dc 100644 --- a/src/include/catalog/toasting.h +++ b/src/include/catalog/toasting.h @@ -51,7 +51,6 @@ extern void BootstrapToastTable(char *relName, /* normal catalogs */ DECLARE_TOAST(pg_aggregate, 4159, 4160); DECLARE_TOAST(pg_attrdef, 2830, 2831); -DECLARE_TOAST(pg_collation, 4161, 4162); DECLARE_TOAST(pg_constraint, 2832, 2833); DECLARE_TOAST(pg_default_acl, 4143, 4144); DECLARE_TOAST(pg_description, 2834, 2835); diff --git a/src/include/commands/collationcmds.h b/src/include/commands/collationcmds.h index 373b85374c..3e1c16ac7f 100644 --- a/src/include/commands/collationcmds.h +++ b/src/include/commands/collationcmds.h @@ -20,6 +20,5 @@ extern ObjectAddress DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_exists); extern void IsThereCollationInNamespace(const char *collname, Oid nspOid); -extern ObjectAddress AlterCollation(AlterCollationStmt *stmt); #endif /* COLLATIONCMDS_H */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 151bcdb7ef..a8da438d22 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -1873,17 +1873,6 @@ typedef struct AlterTableCmd /* one subcommand of an ALTER TABLE */ } AlterTableCmd; -/* ---------------------- - * Alter Collation - * ---------------------- - */ -typedef struct AlterCollationStmt -{ - NodeTag type; - List *collname; -} AlterCollationStmt; - - /* ---------------------- * Alter Domain * diff --git a/src/test/regress/expected/collate.icu.utf8.out b/src/test/regress/expected/collate.icu.utf8.out index 2b86ce9028..60d9263a2f 100644 --- a/src/test/regress/expected/collate.icu.utf8.out +++ b/src/test/regress/expected/collate.icu.utf8.out @@ -1082,9 +1082,6 @@ SELECT collname FROM pg_collation WHERE collname LIKE 'test%'; DROP SCHEMA test_schema; DROP ROLE regress_test_role; --- ALTER -ALTER COLLATION "en-x-icu" REFRESH VERSION; -NOTICE: version has not changed -- dependencies CREATE COLLATION test0 FROM "C"; CREATE TABLE collate_dep_test1 (a int, b text COLLATE test0); diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/regress/expected/collate.linux.utf8.out index f06ae543e4..580b00eea7 100644 --- a/src/test/regress/expected/collate.linux.utf8.out +++ b/src/test/regress/expected/collate.linux.utf8.out @@ -1093,9 +1093,6 @@ SELECT collname FROM pg_collation WHERE collname LIKE 'test%'; DROP SCHEMA test_schema; DROP ROLE regress_test_role; --- ALTER -ALTER COLLATION "en_US" REFRESH VERSION; -NOTICE: version has not changed -- dependencies CREATE COLLATION test0 FROM "C"; CREATE TABLE collate_dep_test1 (a int, b text COLLATE test0); diff --git a/src/test/regress/sql/collate.icu.utf8.sql b/src/test/regress/sql/collate.icu.utf8.sql index 67de7d9794..35acf91fbf 100644 --- a/src/test/regress/sql/collate.icu.utf8.sql +++ b/src/test/regress/sql/collate.icu.utf8.sql @@ -405,11 +405,6 @@ DROP SCHEMA test_schema; DROP ROLE regress_test_role; --- ALTER - -ALTER COLLATION "en-x-icu" REFRESH VERSION; - - -- dependencies CREATE COLLATION test0 FROM "C"; diff --git a/src/test/regress/sql/collate.linux.utf8.sql b/src/test/regress/sql/collate.linux.utf8.sql index cbbd2203e4..c697c99488 100644 --- a/src/test/regress/sql/collate.linux.utf8.sql +++ b/src/test/regress/sql/collate.linux.utf8.sql @@ -406,11 +406,6 @@ DROP SCHEMA test_schema; DROP ROLE regress_test_role; --- ALTER - -ALTER COLLATION "en_US" REFRESH VERSION; - - -- dependencies CREATE COLLATION test0 FROM "C"; -- 2.20.1 --o56keubw4jpehgac Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="v27-0002-Add-pg_depend.refobjversion.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Microsecond-based timeouts @ 2023-03-13 05:23 Thomas Munro <[email protected]> 2023-03-13 21:59 ` Re: Microsecond-based timeouts Andres Freund <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Thomas Munro @ 2023-03-13 05:23 UTC (permalink / raw) To: pgsql-hackers Hi, Over in [1], I thought for a moment that a new function WaitLatchUs(..., timeout_us, ...) was going to be useful to fix that bug report, at least in master, until I realised the required Linux syscall is a little too new (for example RHEL 9 shipped May '22, Debian 12 is expected to be declared "stable" in a few months). So I'm kicking this proof-of-concept over into a new thread to talk about in the next cycle, in case it turns out to be useful later. There probably isn't too much call for very high resolution sleeping. Most time-based sleeping is probably bad, but when it's legitimately used to spread CPU or I/O out (instead of illegitimate use for polling-based algorithms), it seems nice to be able to use all the accuracy your hardware can provide, and yet it is still important to be able to process other kinds of events, so WaitLatchUs() seems like a better building block than pg_usleep(). One question is whether it'd be better to use nanoseconds instead, since the relevant high-resolution primitives use those under the covers (struct timespec). On the other hand, microseconds are a good match for our TimestampTz which is the ultimate source of many of our timeout decisions. I suppose we could also consider an interface with an absolute timeout instead, and then stop thinking about the units so much. As mentioned in that other thread, the only systems that currently seem to be able to sleep less than 1ms through these multiplexing APIs are: Linux 5.11+ (epoll_pwait2()), FreeBSD (kevent()), macOS (ditto). Everything else will round up to milliseconds at the kernel interface (because poll(), epoll_wait() and WaitForMultipleObjects() take those) or later inside the kernel due to kernel tick rounding. There might be ways to do better on Windows with separate timer events, but I don't know. [1] https://www.postgresql.org/message-id/flat/CAAKRu_b-q0hXCBUCAATh0Z4Zi6UkiC0k2DFgoD3nC-r3SkR3tg%40mai... Attachments: [text/x-patch] 0001-Support-microsecond-based-timeouts-in-WaitEventSet-A.patch (16.2K, ../../CA+hUKG+hC9mFx8tEcBsyo7-cAfWgtbRy1eDizeFuff2K7T=4bA@mail.gmail.com/2-0001-Support-microsecond-based-timeouts-in-WaitEventSet-A.patch) download | inline diff: From e99b7d31831f31888a9433a83d3e64ccbe2cc5c7 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Fri, 10 Mar 2023 15:16:47 +1300 Subject: [PATCH 1/3] Support microsecond based timeouts in WaitEventSet API. WaitLatch() can only wait for whole numbers of milliseconds, a limitation inherited ultimately from poll() and similar interfaces. In the past it didn't matter much as sleep times were very inaccurate in practice on common systems, but Linux and others can now be accurate down to small fractions of a millisecond. In order to be able to replace pg_usleep() calls, provide WaitLatchUs(). Just like pg_usleep(), the actual resolution of the sleeping depends on the OS and hardware. For Linux, this requires epoll_pwait2() (Linux 5.11), otherwise we have to round to milliseconds for epoll_wait(). For macOS and *BSD, kevent() has always supported nanosecond-based timeouts, but only macOS and FreeBSD are known to support high resolution timers (other BSDs tested currently round up to kernel ticks so WaitLatch() already couldn't sleep for only 1ms). For Solaris and AIX, we currently use poll() and that requires rounding up to milliseconds, so no improvement over WaitLatch() there. Likewise for Windows (which already couldn't sleep for only 1ms due to internal rounding to tick size). Discussion: https://postgr.es/m/CAAKRu_b-q0hXCBUCAATh0Z4Zi6UkiC0k2DFgoD3nC-r3SkR3tg%40mail.gmail.com --- configure | 2 +- configure.ac | 1 + meson.build | 1 + src/backend/storage/ipc/latch.c | 146 ++++++++++++++++++++++++-------- src/include/pg_config.h.in | 3 + src/include/storage/latch.h | 13 ++- src/tools/msvc/Solution.pm | 1 + 7 files changed, 128 insertions(+), 39 deletions(-) diff --git a/configure b/configure index e35769ea73..914361f91b 100755 --- a/configure +++ b/configure @@ -15699,7 +15699,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in backtrace_symbols copyfile getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l +for ac_func in backtrace_symbols copyfile epoll_pwait2 getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/configure.ac b/configure.ac index af23c15cb2..4249f8002c 100644 --- a/configure.ac +++ b/configure.ac @@ -1794,6 +1794,7 @@ LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` AC_CHECK_FUNCS(m4_normalize([ backtrace_symbols copyfile + epoll_pwait2 getifaddrs getpeerucred inet_pton diff --git a/meson.build b/meson.build index d4384f1bf6..fe9b0470aa 100644 --- a/meson.build +++ b/meson.build @@ -2344,6 +2344,7 @@ func_checks = [ # when enabling asan the dlopen check doesn't notice that -ldl is actually # required. Just checking for dlsym() ought to suffice. ['dlsym', {'dependencies': [dl_dep], 'define': false}], + ['epoll_pwait2'], ['explicit_bzero'], ['fdatasync', {'dependencies': [rt_dep, posix4_dep], 'define': false}], # Solaris ['getifaddrs'], diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c index f4123e7de7..ba9ccb19ac 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -194,7 +194,7 @@ static void WaitEventAdjustPoll(WaitEventSet *set, WaitEvent *event); static void WaitEventAdjustWin32(WaitEventSet *set, WaitEvent *event); #endif -static inline int WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, +static inline int WaitEventSetWaitBlock(WaitEventSet *set, int64 cur_timeout_us, WaitEvent *occurred_events, int nevents); /* @@ -475,10 +475,9 @@ DisownLatch(Latch *latch) * to wait for. If the latch is already set (and WL_LATCH_SET is given), the * function returns immediately. * - * The "timeout" is given in milliseconds. It must be >= 0 if WL_TIMEOUT flag - * is given. Although it is declared as "long", we don't actually support - * timeouts longer than INT_MAX milliseconds. Note that some extra overhead - * is incurred when WL_TIMEOUT is given, so avoid using a timeout if possible. + * The "timeout" is given in microseconds. It must be >= 0 if WL_TIMEOUT flag + * is given. Note that some extra overhead is incurred when WL_TIMEOUT is + * given, so avoid using a timeout if possible. * * The latch must be owned by the current process, ie. it must be a * process-local latch initialized with InitLatch, or a shared latch @@ -489,8 +488,8 @@ DisownLatch(Latch *latch) * we return all of them in one call, but we will return at least one. */ int -WaitLatch(Latch *latch, int wakeEvents, long timeout, - uint32 wait_event_info) +WaitLatchUs(Latch *latch, int wakeEvents, int64 timeout_us, + uint32 wait_event_info) { WaitEvent event; @@ -510,15 +509,32 @@ WaitLatch(Latch *latch, int wakeEvents, long timeout, LatchWaitSet->exit_on_postmaster_death = ((wakeEvents & WL_EXIT_ON_PM_DEATH) != 0); - if (WaitEventSetWait(LatchWaitSet, - (wakeEvents & WL_TIMEOUT) ? timeout : -1, - &event, 1, - wait_event_info) == 0) + if (WaitEventSetWaitUs(LatchWaitSet, + (wakeEvents & WL_TIMEOUT) ? timeout_us : -1, + &event, 1, + wait_event_info) == 0) return WL_TIMEOUT; else return event.events; } +/* + * Like WaitLatchUs(), but with the timeout in milliseconds. + * + * The "timeout" is given in milliseconds. It must be >= 0 if WL_TIMEOUT flag + * is given. Although it is declared as "long", we don't actually support + * timeouts longer than INT_MAX milliseconds. Note that some extra overhead + * is incurred when WL_TIMEOUT is given, so avoid using a timeout if possible. + */ +int +WaitLatch(Latch *latch, int wakeEvents, long timeout_ms, + uint32 wait_event_info) +{ + return WaitLatchUs(latch, wakeEvents, + timeout_ms <= 0 ? timeout_ms : timeout_ms * 1000, + wait_event_info); +} + /* * Like WaitLatch, but with an extra socket argument for WL_SOCKET_* * conditions. @@ -537,8 +553,8 @@ WaitLatch(Latch *latch, int wakeEvents, long timeout, * WaitEventSet instead; that's more efficient. */ int -WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock, - long timeout, uint32 wait_event_info) +WaitLatchOrSocketUs(Latch *latch, int wakeEvents, pgsocket sock, + int64 timeout_us, uint32 wait_event_info) { int ret = 0; int rc; @@ -546,9 +562,9 @@ WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock, WaitEventSet *set = CreateWaitEventSet(CurrentMemoryContext, 3); if (wakeEvents & WL_TIMEOUT) - Assert(timeout >= 0); + Assert(timeout_us >= 0); else - timeout = -1; + timeout_us = -1; if (wakeEvents & WL_LATCH_SET) AddWaitEventToSet(set, WL_LATCH_SET, PGINVALID_SOCKET, @@ -575,7 +591,7 @@ WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock, AddWaitEventToSet(set, ev, sock, NULL, NULL); } - rc = WaitEventSetWait(set, timeout, &event, 1, wait_event_info); + rc = WaitEventSetWaitUs(set, timeout_us, &event, 1, wait_event_info); if (rc == 0) ret |= WL_TIMEOUT; @@ -591,6 +607,20 @@ WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock, return ret; } +/* + * Like WaitLatchOrSocket, but with timeout in milliseconds. + */ +int +WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock, + long timeout_ms, uint32 wait_event_info) +{ + return WaitLatchOrSocketUs(latch, + wakeEvents, + sock, + timeout_ms > 0 ? timeout_ms * 1000 : timeout_ms, + wait_event_info); +} + /* * Sets a latch and wakes up anyone waiting on it. * @@ -1380,14 +1410,14 @@ WaitEventAdjustWin32(WaitEventSet *set, WaitEvent *event) * values associated with the registered event. */ int -WaitEventSetWait(WaitEventSet *set, long timeout, - WaitEvent *occurred_events, int nevents, - uint32 wait_event_info) +WaitEventSetWaitUs(WaitEventSet *set, int64 timeout_us, + WaitEvent *occurred_events, int nevents, + uint32 wait_event_info) { int returned_events = 0; instr_time start_time; instr_time cur_time; - long cur_timeout = -1; + int64 cur_timeout = -1; Assert(nevents > 0); @@ -1395,11 +1425,11 @@ WaitEventSetWait(WaitEventSet *set, long timeout, * Initialize timeout if requested. We must record the current time so * that we can determine the remaining timeout if interrupted. */ - if (timeout >= 0) + if (timeout_us >= 0) { INSTR_TIME_SET_CURRENT(start_time); - Assert(timeout >= 0 && timeout <= INT_MAX); - cur_timeout = timeout; + Assert(timeout_us >= 0); + cur_timeout = timeout_us; } else INSTR_TIME_SET_ZERO(start_time); @@ -1487,11 +1517,11 @@ WaitEventSetWait(WaitEventSet *set, long timeout, returned_events = rc; /* If we're not done, update cur_timeout for next iteration */ - if (returned_events == 0 && timeout >= 0) + if (returned_events == 0 && timeout_us >= 0) { INSTR_TIME_SET_CURRENT(cur_time); INSTR_TIME_SUBTRACT(cur_time, start_time); - cur_timeout = timeout - (long) INSTR_TIME_GET_MILLISEC(cur_time); + cur_timeout = timeout_us - INSTR_TIME_GET_MICROSEC(cur_time); if (cur_timeout <= 0) break; } @@ -1505,6 +1535,20 @@ WaitEventSetWait(WaitEventSet *set, long timeout, return returned_events; } +/* + * Like WaitEventSetWaitUs(), but the timeout specified in milliseconds. + */ +int +WaitEventSetWait(WaitEventSet *set, long timeout_ms, + WaitEvent *occurred_events, int nevents, + uint32 wait_event_info) +{ + return WaitEventSetWaitUs(set, + timeout_ms <= 0 ? timeout_ms : timeout_ms * 1000, + occurred_events, + nevents, + wait_event_info); +} #if defined(WAIT_USE_EPOLL) @@ -1517,17 +1561,31 @@ WaitEventSetWait(WaitEventSet *set, long timeout, * easy. */ static inline int -WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, +WaitEventSetWaitBlock(WaitEventSet *set, int64 cur_timeout_us, WaitEvent *occurred_events, int nevents) { int returned_events = 0; int rc; WaitEvent *cur_event; struct epoll_event *cur_epoll_event; +#ifdef HAVE_EPOLL_PWAIT2 + struct timespec nap; +#endif /* Sleep */ +#ifdef HAVE_EPOLL_PWAIT2 + nap.tv_sec = cur_timeout_us / 1000000; + nap.tv_nsec = (cur_timeout_us % 1000000) * 1000; + rc = epoll_pwait2(set->epoll_fd, set->epoll_ret_events, + Min(nevents, set->nevents_space), + cur_timeout_us >= 0 ? &nap : NULL, + NULL); +#else rc = epoll_wait(set->epoll_fd, set->epoll_ret_events, - Min(nevents, set->nevents_space), cur_timeout); + Min(nevents, set->nevents_space), + cur_timeout_us >= 0 ? (cur_timeout_us + 999) / 1000 + : cur_timeout_us); +#endif /* Check return code */ if (rc < 0) @@ -1653,7 +1711,7 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, * with separate system calls. */ static int -WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, +WaitEventSetWaitBlock(WaitEventSet *set, int64 cur_timeout_us, WaitEvent *occurred_events, int nevents) { int returned_events = 0; @@ -1663,12 +1721,12 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, struct timespec timeout; struct timespec *timeout_p; - if (cur_timeout < 0) + if (cur_timeout_us < 0) timeout_p = NULL; else { - timeout.tv_sec = cur_timeout / 1000; - timeout.tv_nsec = (cur_timeout % 1000) * 1000000; + timeout.tv_sec = cur_timeout_us / 1000000; + timeout.tv_nsec = (cur_timeout_us % 1000000) * 1000; timeout_p = &timeout; } @@ -1806,16 +1864,25 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, * but requires iterating through all of set->pollfds. */ static inline int -WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, +WaitEventSetWaitBlock(WaitEventSet *set, int64 cur_timeout_us, WaitEvent *occurred_events, int nevents) { int returned_events = 0; int rc; WaitEvent *cur_event; struct pollfd *cur_pollfd; + int cur_timeout_ms; + + /* Round up to the nearest millisecond, and cap at INT_MAX. */ + if (cur_timeout_us >= PG_INT64_MAX - 999) + cur_timeout_ms = INT_MAX; + else if (cur_timeout_us > 0) + cur_timeout_ms = Min((int64) INT_MAX, (cur_timeout_us + 999) / 1000); + else + cur_timeout_ms = cur_timeout_us; /* Sleep */ - rc = poll(set->pollfds, set->nevents, (int) cur_timeout); + rc = poll(set->pollfds, set->nevents, cur_timeout_ms); /* Check return code */ if (rc < 0) @@ -1943,12 +2010,21 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, * that only one event is "consumed". */ static inline int -WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, +WaitEventSetWaitBlock(WaitEventSet *set, int64 cur_timeout_us, WaitEvent *occurred_events, int nevents) { int returned_events = 0; DWORD rc; WaitEvent *cur_event; + int cur_timeout_ms; + + /* Round up to the nearest millisecond, and cap at INT_MAX. */ + if (cur_timeout_us >= PG_INT64_MAX - 999) + cur_timeout_ms = INT_MAX; + else if (cur_timeout_us > 0) + cur_timeout_ms = Min((int64) INT_MAX, (cur_timeout_us + 999) / 1000); + else + cur_timeout_ms = cur_timeout_us; /* Reset any wait events that need it */ for (cur_event = set->events; @@ -2000,7 +2076,7 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, * Need to wait for ->nevents + 1, because signal handle is in [0]. */ rc = WaitForMultipleObjects(set->nevents + 1, set->handles, FALSE, - cur_timeout); + cur_timeout_ms); /* Check return code */ if (rc == WAIT_FAILED) diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 20c82f5979..c1f1fc6e70 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -149,6 +149,9 @@ /* Define to 1 if you have the <editline/readline.h> header file. */ #undef HAVE_EDITLINE_READLINE_H +/* Define to 1 if you have the `epoll_pwait2' function. */ +#undef HAVE_EPOLL_PWAIT2 + /* Define to 1 if you have the <execinfo.h> header file. */ #undef HAVE_EXECINFO_H diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h index 99cc47874a..756c3114ed 100644 --- a/src/include/storage/latch.h +++ b/src/include/storage/latch.h @@ -180,13 +180,20 @@ extern int AddWaitEventToSet(WaitEventSet *set, uint32 events, pgsocket fd, Latch *latch, void *user_data); extern void ModifyWaitEvent(WaitEventSet *set, int pos, uint32 events, Latch *latch); -extern int WaitEventSetWait(WaitEventSet *set, long timeout, +extern int WaitEventSetWait(WaitEventSet *set, long timeout_ms, WaitEvent *occurred_events, int nevents, uint32 wait_event_info); -extern int WaitLatch(Latch *latch, int wakeEvents, long timeout, +extern int WaitEventSetWaitUs(WaitEventSet *set, int64 timeout_us, + WaitEvent *occurred_events, int nevents, + uint32 wait_event_info); +extern int WaitLatch(Latch *latch, int wakeEvents, long timeout_ms, uint32 wait_event_info); +extern int WaitLatchUs(Latch *latch, int wakeEvents, int64 timeout_us, + uint32 wait_event_info); extern int WaitLatchOrSocket(Latch *latch, int wakeEvents, - pgsocket sock, long timeout, uint32 wait_event_info); + pgsocket sock, long timeout_ms, uint32 wait_event_info); +extern int WaitLatchOrSocketUs(Latch *latch, int wakeEvents, + pgsocket sock, int64 timeout_us, uint32 wait_event_info); extern void InitializeLatchWaitSet(void); extern int GetNumRegisteredWaitEvents(WaitEventSet *set); extern bool WaitEventSetCanReportClosed(void); diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index 5eaea6355e..f88fffa5e2 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -247,6 +247,7 @@ sub GenerateFiles HAVE_DECL_STRNLEN => 1, HAVE_EDITLINE_HISTORY_H => undef, HAVE_EDITLINE_READLINE_H => undef, + HAVE_EPOLL_PWAIT2 => undef, HAVE_EXECINFO_H => undef, HAVE_EXPLICIT_BZERO => undef, HAVE_FSEEKO => 1, -- 2.39.2 [text/x-patch] 0002-Use-microsecond-based-naps-for-vacuum_cost_delay-sle.patch (1.4K, ../../CA+hUKG+hC9mFx8tEcBsyo7-cAfWgtbRy1eDizeFuff2K7T=4bA@mail.gmail.com/3-0002-Use-microsecond-based-naps-for-vacuum_cost_delay-sle.patch) download | inline diff: From 9b5e8922b7c603f902fecfb30e24a962b6c08176 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Fri, 10 Mar 2023 16:22:59 +1300 Subject: [PATCH 2/3] Use microsecond-based naps for vacuum_cost_delay sleep. Now that we have microsecond support in the WaitEventSet API, we can use the standard programming pattern to implement the high resolution sleep in vacuum_delay_point(). XXX We wouldn't be able to do this until Linux 5.11 is in common stable distributions, otherwise the sleep would lose precision when changing from the pg_usleep() coding. Reported-by: Melanie Plageman <[email protected]> Discussion: https://postgr.es/m/CAAKRu_b-q0hXCBUCAATh0Z4Zi6UkiC0k2DFgoD3nC-r3SkR3tg%40mail.gmail.com --- src/backend/commands/vacuum.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 2e12baf8eb..f379e60dca 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -2232,10 +2232,10 @@ vacuum_delay_point(void) if (msec > VacuumCostDelay * 4) msec = VacuumCostDelay * 4; - (void) WaitLatch(MyLatch, - WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, - msec, - WAIT_EVENT_VACUUM_DELAY); + (void) WaitLatchUs(MyLatch, + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, + msec * 1000, + WAIT_EVENT_VACUUM_DELAY); ResetLatch(MyLatch); VacuumCostBalance = 0; -- 2.39.2 [text/x-patch] 0003-Use-microsecond-based-naps-in-walreceiver.patch (3.7K, ../../CA+hUKG+hC9mFx8tEcBsyo7-cAfWgtbRy1eDizeFuff2K7T=4bA@mail.gmail.com/4-0003-Use-microsecond-based-naps-in-walreceiver.patch) download | inline diff: From dd40a3e3c28c69466bc2e8c2a223608ac51e05b7 Mon Sep 17 00:00:00 2001 From: Thomas Munro <[email protected]> Date: Fri, 10 Mar 2023 16:19:42 +1300 Subject: [PATCH 3/3] Use microsecond-based naps in walreceiver. Since anything based on timestamp differences is really in microseconds under the covers, we might as well use the new higher resolution API for waiting. XXX For illustration; there would be many other places that could change like this --- src/backend/replication/walreceiver.c | 16 ++++++++-------- src/backend/utils/adt/timestamp.c | 20 ++++++++++++++++++++ src/include/utils/timestamp.h | 2 ++ 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c index f6446da2d6..18c66c0c63 100644 --- a/src/backend/replication/walreceiver.c +++ b/src/backend/replication/walreceiver.c @@ -445,7 +445,7 @@ WalReceiverMain(void) pgsocket wait_fd = PGINVALID_SOCKET; int rc; TimestampTz nextWakeup; - long nap; + int64 nap; /* * Exit walreceiver if we're not in recovery. This should not @@ -530,7 +530,7 @@ WalReceiverMain(void) /* Calculate the nap time, clamping as necessary. */ now = GetCurrentTimestamp(); - nap = TimestampDifferenceMilliseconds(now, nextWakeup); + nap = TimestampDifferenceMicroseconds(now, nextWakeup); /* * Ideally we would reuse a WaitEventSet object repeatedly @@ -544,12 +544,12 @@ WalReceiverMain(void) * avoiding some system calls. */ Assert(wait_fd != PGINVALID_SOCKET); - rc = WaitLatchOrSocket(MyLatch, - WL_EXIT_ON_PM_DEATH | WL_SOCKET_READABLE | - WL_TIMEOUT | WL_LATCH_SET, - wait_fd, - nap, - WAIT_EVENT_WAL_RECEIVER_MAIN); + rc = WaitLatchOrSocketUs(MyLatch, + WL_EXIT_ON_PM_DEATH | WL_SOCKET_READABLE | + WL_TIMEOUT | WL_LATCH_SET, + wait_fd, + nap, + WAIT_EVENT_WAL_RECEIVER_MAIN); if (rc & WL_LATCH_SET) { ResetLatch(MyLatch); diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c index de93db89d4..52f6568397 100644 --- a/src/backend/utils/adt/timestamp.c +++ b/src/backend/utils/adt/timestamp.c @@ -1719,6 +1719,26 @@ TimestampDifferenceMilliseconds(TimestampTz start_time, TimestampTz stop_time) return (long) ((diff + 999) / 1000); } +/* + * TimestampDifferenceMicroseconds -- convert the difference between two + * timestamps into microseconds + * + * Compute a wait time for WaitLatchUs(). + */ +int64 +TimestampDifferenceMicroseconds(TimestampTz start_time, TimestampTz stop_time) +{ + TimestampTz diff; + + /* Deal with zero or negative elapsed time quickly. */ + if (start_time >= stop_time) + return 0; + /* To not fail with timestamp infinities, we must detect overflow. */ + if (pg_sub_s64_overflow(stop_time, start_time, &diff)) + return PG_INT64_MAX; + return diff; +} + /* * TimestampDifferenceExceeds -- report whether the difference between two * timestamps is >= a threshold (expressed in milliseconds) diff --git a/src/include/utils/timestamp.h b/src/include/utils/timestamp.h index edd59dc432..1caa15221d 100644 --- a/src/include/utils/timestamp.h +++ b/src/include/utils/timestamp.h @@ -100,6 +100,8 @@ extern void TimestampDifference(TimestampTz start_time, TimestampTz stop_time, long *secs, int *microsecs); extern long TimestampDifferenceMilliseconds(TimestampTz start_time, TimestampTz stop_time); +extern int64 TimestampDifferenceMicroseconds(TimestampTz start_time, + TimestampTz stop_time); extern bool TimestampDifferenceExceeds(TimestampTz start_time, TimestampTz stop_time, int msec); -- 2.39.2 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Microsecond-based timeouts 2023-03-13 05:23 Microsecond-based timeouts Thomas Munro <[email protected]> @ 2023-03-13 21:59 ` Andres Freund <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Andres Freund @ 2023-03-13 21:59 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: pgsql-hackers Hi, On 2023-03-13 18:23:02 +1300, Thomas Munro wrote: > One question is whether it'd be better to use nanoseconds instead, > since the relevant high-resolution primitives use those under the > covers (struct timespec). On the other hand, microseconds are a good > match for our TimestampTz which is the ultimate source of many of our > timeout decisions. It's hard to believe we'll need nanosecond sleeps anytime soon, given that even very trivial syscalls take on the order of 100ns. It's not like we couldn't add another function for waiting for nanoseconds at a later point. > I suppose we could also consider an interface with an absolute timeout > instead, and then stop thinking about the units so much. That seesm pretty awful to use, and we'd just end up with the same question at the callsites. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-03-13 21:59 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2019-05-28 18:15 [PATCH v27 1/5] Remove pg_collation.collversion. Thomas Munro <[email protected]> 2023-03-13 05:23 Microsecond-based timeouts Thomas Munro <[email protected]> 2023-03-13 21:59 ` Re: Microsecond-based timeouts Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox