On 2020/01/24 15:38, Arthur Zakirov wrote:
On 2020/01/24 14:56, Michael Paquier wrote:
On Fri, Jan 24, 2020 at 01:28:29PM +0900, Arthur Zakirov wrote:
It is compiled and passes the tests. There is the documentation and
it is
built too without an error.
It seems that consensus about the returned type was reached and I
marked the
patch as "Ready for Commiter".
+ fsync_fname_ext(filename, S_ISDIR(fst.st_mode), false, ERROR);
One comment here: should we warn better users in the docs that a fsync
failule will not trigger a PANIC here? Here, fsync failure on heap
file => ERROR => potential data corruption.
Ah, true. It is possible to add couple sentences that pg_file_sync()
doesn't depend on data_sync_retry GUC and doesn't raise a PANIC even for
database files.
Thanks all for the review!
So, what about the attached patch?
In the patch, I added the following note to the doc.
--------------------
Note that
<xref linkend="guc-data-sync-retry"/> has no effect on this function,
and therefore a PANIC-level error will not be raised even on failure to
flush database files.
--------------------
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
diff --git a/contrib/adminpack/Makefile b/contrib/adminpack/Makefile
index 9a464b11bc..630fea7726 100644
--- a/contrib/adminpack/Makefile
+++ b/contrib/adminpack/Makefile
@@ -7,7 +7,8 @@ OBJS = \
PG_CPPFLAGS = -I$(libpq_srcdir)
EXTENSION = adminpack
-DATA = adminpack--1.0.sql adminpack--1.0--1.1.sql adminpack--1.1--2.0.sql
+DATA = adminpack--1.0.sql adminpack--1.0--1.1.sql adminpack--1.1--2.0.sql\
+ adminpack--2.0--2.1.sql
PGFILEDESC = "adminpack - support functions for pgAdmin"
REGRESS = adminpack
diff --git a/contrib/adminpack/adminpack--2.0--2.1.sql
b/contrib/adminpack/adminpack--2.0--2.1.sql
new file mode 100644
index 0000000000..1c6712e816
--- /dev/null
+++ b/contrib/adminpack/adminpack--2.0--2.1.sql
@@ -0,0 +1,17 @@
+/* contrib/adminpack/adminpack--2.0--2.1.sql */
+
+-- complain if script is sourced in psql, rather than via ALTER EXTENSION
+\echo Use "ALTER EXTENSION adminpack UPDATE TO '2.1'" to load this file. \quit
+
+/* ***********************************************
+ * Administrative functions for PostgreSQL
+ * *********************************************** */
+
+/* generic file access functions */
+
+CREATE OR REPLACE FUNCTION pg_catalog.pg_file_sync(text)
+RETURNS void
+AS 'MODULE_PATHNAME', 'pg_file_sync'
+LANGUAGE C VOLATILE STRICT;
+
+REVOKE EXECUTE ON FUNCTION pg_catalog.pg_file_sync(text) FROM PUBLIC;
diff --git a/contrib/adminpack/adminpack.c b/contrib/adminpack/adminpack.c
index 710f4ea32d..7b5a531e08 100644
--- a/contrib/adminpack/adminpack.c
+++ b/contrib/adminpack/adminpack.c
@@ -43,6 +43,7 @@ PG_MODULE_MAGIC;
PG_FUNCTION_INFO_V1(pg_file_write);
PG_FUNCTION_INFO_V1(pg_file_write_v1_1);
+PG_FUNCTION_INFO_V1(pg_file_sync);
PG_FUNCTION_INFO_V1(pg_file_rename);
PG_FUNCTION_INFO_V1(pg_file_rename_v1_1);
PG_FUNCTION_INFO_V1(pg_file_unlink);
@@ -215,6 +216,30 @@ pg_file_write_internal(text *file, text *data, bool
replace)
return (count);
}
+/* ------------------------------------
+ * pg_file_sync
+ *
+ * We REVOKE EXECUTE on the function from PUBLIC.
+ * Users can then grant access to it based on their policies.
+ */
+Datum
+pg_file_sync(PG_FUNCTION_ARGS)
+{
+ char *filename;
+ struct stat fst;
+
+ filename = convert_and_check_filename(PG_GETARG_TEXT_PP(0), false);
+
+ if (stat(filename, &fst) < 0)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not stat file \"%s\": %m",
filename)));
+
+ fsync_fname_ext(filename, S_ISDIR(fst.st_mode), false, ERROR);
+
+ PG_RETURN_VOID();
+}
+
/* ------------------------------------
* pg_file_rename - old version
*
diff --git a/contrib/adminpack/adminpack.control
b/contrib/adminpack/adminpack.control
index 12569dcdd7..ae35d22156 100644
--- a/contrib/adminpack/adminpack.control
+++ b/contrib/adminpack/adminpack.control
@@ -1,6 +1,6 @@
# adminpack extension
comment = 'administrative functions for PostgreSQL'
-default_version = '2.0'
+default_version = '2.1'
module_pathname = '$libdir/adminpack'
relocatable = false
schema = pg_catalog
diff --git a/contrib/adminpack/expected/adminpack.out
b/contrib/adminpack/expected/adminpack.out
index 8747ac69a2..5738b0f6c4 100644
--- a/contrib/adminpack/expected/adminpack.out
+++ b/contrib/adminpack/expected/adminpack.out
@@ -56,6 +56,21 @@ RESET ROLE;
REVOKE EXECUTE ON FUNCTION pg_file_write(text,text,bool) FROM regress_user1;
REVOKE pg_read_all_settings FROM regress_user1;
DROP ROLE regress_user1;
+-- sync
+SELECT pg_file_sync('test_file1'); -- sync file
+ pg_file_sync
+--------------
+
+(1 row)
+
+SELECT pg_file_sync('pg_stat'); -- sync directory
+ pg_file_sync
+--------------
+
+(1 row)
+
+SELECT pg_file_sync('test_file2'); -- not there
+ERROR: could not stat file "test_file2": No such file or directory
-- rename file
SELECT pg_file_rename('test_file1', 'test_file2');
pg_file_rename
@@ -142,6 +157,8 @@ CREATE USER regress_user1;
SET ROLE regress_user1;
SELECT pg_file_write('test_file0', 'test0', false);
ERROR: permission denied for function pg_file_write
+SELECT pg_file_sync('test_file0');
+ERROR: permission denied for function pg_file_sync
SELECT pg_file_rename('test_file0', 'test_file0');
ERROR: permission denied for function pg_file_rename
CONTEXT: SQL function "pg_file_rename" statement 1
diff --git a/contrib/adminpack/sql/adminpack.sql
b/contrib/adminpack/sql/adminpack.sql
index 1525f0a82b..918d0bdc65 100644
--- a/contrib/adminpack/sql/adminpack.sql
+++ b/contrib/adminpack/sql/adminpack.sql
@@ -29,6 +29,11 @@ REVOKE EXECUTE ON FUNCTION pg_file_write(text,text,bool)
FROM regress_user1;
REVOKE pg_read_all_settings FROM regress_user1;
DROP ROLE regress_user1;
+-- sync
+SELECT pg_file_sync('test_file1'); -- sync file
+SELECT pg_file_sync('pg_stat'); -- sync directory
+SELECT pg_file_sync('test_file2'); -- not there
+
-- rename file
SELECT pg_file_rename('test_file1', 'test_file2');
SELECT pg_read_file('test_file1'); -- not there
@@ -58,6 +63,7 @@ CREATE USER regress_user1;
SET ROLE regress_user1;
SELECT pg_file_write('test_file0', 'test0', false);
+SELECT pg_file_sync('test_file0');
SELECT pg_file_rename('test_file0', 'test_file0');
SELECT pg_file_unlink('test_file0');
SELECT pg_logdir_ls();
diff --git a/doc/src/sgml/adminpack.sgml b/doc/src/sgml/adminpack.sgml
index 2655417366..977073f7c8 100644
--- a/doc/src/sgml/adminpack.sgml
+++ b/doc/src/sgml/adminpack.sgml
@@ -43,6 +43,13 @@
Write, or append to, a text file
</entry>
</row>
+ <row>
+ <entry><function>pg_catalog.pg_file_sync(filename text)</function></entry>
+ <entry><type>void</type></entry>
+ <entry>
+ Flush a file or directory to disk
+ </entry>
+ </row>
<row>
<entry><function>pg_catalog.pg_file_rename(oldname text, newname text
<optional>, archivename text</optional>)</function></entry>
<entry><type>boolean</type></entry>
@@ -79,6 +86,18 @@
Returns the number of bytes written.
</para>
+ <indexterm>
+ <primary>pg_file_sync</primary>
+ </indexterm>
+ <para>
+ <function>pg_file_sync</function> fsyncs the specified file or directory
+ named by <parameter>filename</parameter>. An error is thrown
+ on failure (e.g., the specified file is not present). Note that
+ <xref linkend="guc-data-sync-retry"/> has no effect on this function,
+ and therefore a PANIC-level error will not be raised even on failure to
+ flush database files.
+ </para>
+
<indexterm>
<primary>pg_file_rename</primary>
</indexterm>
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index fa79b45f63..b5f4df6a48 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -319,7 +319,6 @@ static void pre_sync_fname(const char *fname, bool isdir,
int elevel);
static void datadir_fsync_fname(const char *fname, bool isdir, int elevel);
static void unlink_if_exists_fname(const char *fname, bool isdir, int elevel);
-static int fsync_fname_ext(const char *fname, bool isdir, bool
ignore_perm, int elevel);
static int fsync_parent_path(const char *fname, int elevel);
@@ -3376,7 +3375,7 @@ unlink_if_exists_fname(const char *fname, bool isdir, int
elevel)
*
* Returns 0 if the operation succeeded, -1 otherwise.
*/
-static int
+int
fsync_fname_ext(const char *fname, bool isdir, bool ignore_perm, int elevel)
{
int fd;
diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index c6ce7eacf2..51e2ece3c9 100644
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -145,6 +145,7 @@ extern int pg_fsync_writethrough(int fd);
extern int pg_fdatasync(int fd);
extern void pg_flush_data(int fd, off_t offset, off_t amount);
extern void fsync_fname(const char *fname, bool isdir);
+extern int fsync_fname_ext(const char *fname, bool isdir, bool
ignore_perm, int elevel);
extern int durable_rename(const char *oldfile, const char *newfile, int
loglevel);
extern int durable_unlink(const char *fname, int loglevel);
extern int durable_link_or_rename(const char *oldfile, const char
*newfile, int loglevel);
