date:20200116

Re: [PATCH v1] pg_ls_tmpdir to show directories

2020-01-16 Thread Fabien COELHO




Hello Justin,


I'm trying to think about how to get rid of the strange structure and hacks,
and the arbitrary looking size 2 array.

Also the recursion is one step, but I'm not sure why, ISTM it could/should
go on always?


Because tmpfiles only go one level deep.


I'm not sure it is a general rule. ISTM that extensions can use tmp files, 
and we would have no control about what they would do there.



Looking at the code, ISTM that relying on a stack/list would be much cleaner
and easier to understand. The code could look like:


I'm willing to change the implementation, but only after there's an agreement
about the desired behavior (extra column, one level, etc).


For the level, ISTM that the implementation should not make this 
assumption. If in practice there is just one level, then the function will 
not recurse deep, no problem.


For the column, I'm not sure that "isdir" is necessary.

You could put it implicitely in the file name by ending it with "/", 
and/or showing the directory contents is enough a hint that there is a 
directory?


Also, I'm not fully sure why ".*" files should be skipped, maybe it should 
be an option? Or the user can filter it with SQL if it does not want them?


--
Fabien.

Re: Setting min/max TLS protocol in clientside libpq

2020-01-16 Thread Daniel Gustafsson

> On 16 Jan 2020, at 04:22, Michael Paquier  wrote:
> 
> On Wed, Jan 15, 2020 at 02:58:09PM +0900, Michael Paquier wrote:
>> On Tue, Jan 14, 2020 at 11:01:00PM +0100, Daniel Gustafsson wrote:
>>> Files renamed to match existing naming convention, the rest of the patch 
>>> left
>>> unchanged.
>> 
>> [previous review]
> 
> One thing I remembered after sleeping on it is that we can split the
> patch into two parts: the refactoring pieces and the addition of the
> options for libpq.

Correct, they are mostly independent (the refactoring doesn't make a lot of
sense without the follow-up patch, but the min/max patch can be kept more
readable without the refactoring in it as well).

> The previous review mostly impacts the libpq part,
> and the split is straight-forward, so attached is a patch for only the
> refactoring pieces with some fixes and tweaks.  I have tested it with
> and without OpenSSL, using 1.0.2 and 1.1.0 on Linux and Windows
> (MSVC).  Those tests have allowed me to find an error in the previous
> patch that I missed: the new files openssl.h and protocol_openssl.c
> still declared SSL_CTX_set_min/max_proto_version as static functions,
> so compilation was broken when trying to use OpenSSL <= 1.0.2.

Doh .. thanks.

> If that looks fine, I would like to get that part committed first.
> Daniel, any thoughts?

The patch looks fine to me, I don't an issue with splitting it into a
refactoring patch and a TLS min/max version patch.

cheers ./daniel

Re: Option to dump foreign data in pg_dump

2020-01-16 Thread vignesh C

On Tue, Jan 14, 2020 at 5:22 PM Luis Carril  wrote:

> Can you have a look at dump with parallel option. Parallel option will
> take a lock on table while invoking lockTableForWorker. May be this is
> not required for foreign tables.
> Thoughts?
>
> I tried with -j and found no issue. I guess that the foreign table needs
> locking anyway to prevent anyone to modify it while is being dumped.
>
>
I'm able to get the problem with the following steps:
Bring up a postgres setup with servers running in 5432 & 5433 port.

Execute the following commands in Server1 configured on 5432 port:

   - CREATE EXTENSION postgres_fdw;

   - CREATE SERVER foreign_server FOREIGN DATA WRAPPER postgres_fdw OPTIONS
   (host '127.0.0.1', port '5433', dbname 'postgres');

   - create user user1 password '123';

   - alter user user1 with superuser;

   - CREATE USER MAPPING FOR user1 SERVER foreign_server OPTIONS (user
   'user1', password '123');

Execute the following commands in Server2 configured on 5433 port:

   - create user user1 password '123';

   - alter user user1 with superuser;

Execute the following commands in Server2 configured on 5433 port as user1
user:

   - create schema test;

   - create table test.test1(id int);

   - insert into test.test1 values(10);

Execute the following commands in Server1 configured on 5432 port as user1
user:

   - CREATE FOREIGN TABLE foreign_table1 (id integer NOT NULL) SERVER
   foreign_server OPTIONS (schema_name 'test', table_name 'test1');

Without parallel option, the operation is successful:

   - ./pg_dump -d postgres -f dumpdir -U user1 -F d  --include-foreign-data
   foreign_server

With parallel option it fails:

   - ./pg_dump -d postgres -f dumpdir1 -U user1 -F d -j 5
   --include-foreign-data foreign_server

pg_dump: error: could not obtain lock on relation "public.foreign_table1"
This usually means that someone requested an ACCESS EXCLUSIVE lock on the
table after the pg_dump parent process had gotten the initial ACCESS SHARE
lock on the table.
pg_dump: error: a worker process died unexpectedly

There may be simpler steps than this to reproduce the issue, i have not try
to optimize it.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

Re: Amcheck: do rightlink verification with lock coupling

2020-01-16 Thread Andrey Borodin




> 14 янв. 2020 г., в 9:47, Andrey Borodin  написал(а):
> 
> Page updates may be lost due to bug in backup software with incremental 
> backups, bug in storage layer of Aurora-style system, bug in page cache, 
> incorrect
> fsync error handling, bug in ssd firmware etc. And our data checksums do not
> detect this kind of corruption. BTW I think that it would be better if our
> checksums were not stored on a page itseft, they could detect this kind of 
> faults.

Observed it just now.
There is one HA cluster where a node was marked dead. This node was 
disconnected from cluster, but due to human error there was postgres running.
Node managed to install block-level incremental backup to the chain. And backup 
software did not detect that backup step was taken from part of timeline that 
was not in actual timeline's history.
Result of restoration is:

man-w%/%db R # select bt_index_check('%.pk_%');
 bt_index_check 

 
(1 row)

Time: 1411.065 ms (00:01.411)
man-w%/%db R # select patched_index_check('%.pk_%');
ERROR:  XX002: left link/right link pair in index "pk_labels" not in agreement
DETAIL:  Block=42705 left block=42707 left link from block=45495.
LOCATION:  bt_recheck_block_rightlink, verify_nbtree.c:621
Time: 671.336 ms

('%' is replacing removed chars)

I understand that this corruption was not introduced by postgres itself, but by 
combination of bug in two 3rd party tools and human error.
But I can imagine similar corruptions with different root causes.

Best regards, Andrey Borodin.

Re: Implementing Incremental View Maintenance

2020-01-16 Thread nuko yokohama

Error occurs when updating user-defined type columns.

Create an INCREMENTAL MATERIALIZED VIEW by specifying a query that includes
user-defined type columns.
After the view is created, an error occurs when inserting into the view
source table (including the user-defined type column).
```
ERROR:  operator does not exist
```

An execution example is shown below.

```
[ec2-user@ip-10-0-1-10 ivm]$ psql testdb -a -f extension-insert.sql
--
-- pg_fraction: https://github.com/nuko-yokohama/pg_fraction
--
DROP EXTENSION IF EXISTS pg_fraction CASCADE;
psql:extension-insert.sql:4: NOTICE:  drop cascades to column data of table
foo
DROP EXTENSION
DROP TABLE IF EXISTS foo CASCADE;
DROP TABLE
CREATE EXTENSION IF NOT EXISTS pg_fraction;
CREATE EXTENSION
\dx
   List of installed extensions
Name | Version |   Schema   | Description
-+-++--
 pg_fraction | 1.0 | public | fraction data type
 plpgsql | 1.0 | pg_catalog | PL/pgSQL procedural language
(2 rows)

\dT+ fraction
List of data types
 Schema |   Name   | Internal name | Size | Elements |  Owner   | Access
privileges | Description
+--+---+--+--+--+---+-
 public | fraction | fraction  | 16   |  | postgres |
|
(1 row)

CREATE TABLE foo (id int, data fraction);
CREATE TABLE
INSERT INTO foo (id, data) VALUES (1,'2/3'),(2,'1/3'),(3,'1/2');
INSERT 0 3
SELECT id, data FROM foo WHERE data >= '1/2';
 id | data
+--
  1 | 2/3
  3 | 1/2
(2 rows)

CREATE INCREMENTAL MATERIALIZED VIEW foo_imv AS SELECT id, data FROM foo
WHERE data >= '1/2';
SELECT 2
TABLE foo_imv;
 id | data
+--
  1 | 2/3
  3 | 1/2
(2 rows)

INSERT INTO foo (id, data) VALUES (4,'2/3'),(5,'2/5'),(6,'3/6'); -- error
psql:extension-insert.sql:17: ERROR:  operator does not exist: fraction
pg_catalog.= fraction
LINE 1: ...(mv.id IS NULL AND diff.id IS NULL)) AND (mv.data OPERATOR(p...
 ^
HINT:  No operator matches the given name and argument types. You might
need to add explicit type casts.
QUERY:  WITH updt AS (UPDATE public.foo_imv AS mv SET __ivm_count__ =
mv.__ivm_count__ OPERATOR(pg_catalog.+) diff.__ivm_count__  FROM
pg_temp_3.pg_temp_73900 AS diff WHERE (mv.id OPERATOR(pg_catalog.=) diff.id
OR (mv.id IS NULL AND diff.id IS NULL)) AND (mv.data OPERATOR(pg_catalog.=)
diff.data OR (mv.data IS NULL AND diff.data IS NULL)) RETURNING mv.id,
mv.data) INSERT INTO public.foo_imv SELECT * FROM pg_temp_3.pg_temp_73900
AS diff WHERE NOT EXISTS (SELECT 1 FROM updt AS mv WHERE (mv.id
OPERATOR(pg_catalog.=) diff.id OR (mv.id IS NULL AND diff.id IS NULL)) AND
(mv.data OPERATOR(pg_catalog.=) diff.data OR (mv.data IS NULL AND diff.data
IS NULL)));
TABLE foo;
 id | data
+--
  1 | 2/3
  2 | 1/3
  3 | 1/2
(3 rows)

TABLE foo_imv;
 id | data
+--
  1 | 2/3
  3 | 1/2
(2 rows)

DROP MATERIALIZED VIEW foo_imv;
DROP MATERIALIZED VIEW
INSERT INTO foo (id, data) VALUES (4,'2/3'),(5,'2/5'),(6,'3/6');
INSERT 0 3
TABLE foo;
 id | data
+--
  1 | 2/3
  2 | 1/3
  3 | 1/2
  4 | 2/3
  5 | 2/5
  6 | 1/2
(6 rows)

```

Best regards.

2018年12月27日(木) 21:57 Yugo Nagata :

> Hi,
>
> I would like to implement Incremental View Maintenance (IVM) on
> PostgreSQL.
> IVM is a technique to maintain materialized views which computes and
> applies
> only the incremental changes to the materialized views rather than
> recomputate the contents as the current REFRESH command does.
>
> I had a presentation on our PoC implementation of IVM at PGConf.eu 2018
> [1].
> Our implementation uses row OIDs to compute deltas for materialized
> views.
> The basic idea is that if we have information about which rows in base
> tables
> are contributing to generate a certain row in a matview then we can
> identify
> the affected rows when a base table is updated. This is based on an idea of
> Dr. Masunaga [2] who is a member of our group and inspired from ID-based
> approach[3].
>
> In our implementation, the mapping of the row OIDs of the materialized view
> and the base tables are stored in "OID map". When a base relation is
> modified,
> AFTER trigger is executed and the delta is recorded in delta tables using
> the transition table feature. The accual udpate of the matview is triggerd
> by REFRESH command with INCREMENTALLY option.
>
> However, we realize problems of our implementation. First, WITH OIDS will
> be removed since PG12, so OIDs are no longer available. Besides this, it
> would
> be hard to implement this since it needs many changes of executor nodes to
> collect base tables's OIDs during execuing a query. Also, the cost of
> maintaining
> OID map would be high.
>
> For these reasons, we started to think to implement IVM without relying on
> OIDs
> and made a bit more surveys.
>
> We also looked at Kevin Grittner's discussion [4]

empty range

2020-01-16 Thread david . turon

Hello,

default constructor for ranges use lower bound closed '[' and upper bound 
open ')'. This is correct behavior, but when upper bound is same like 
lower bound then range is empty. Mathematically is correct again - but in 
database is lost information about range bounds (lower/upper is NULL). To 
prevent this sitiuation we must have check if lower and upper argument is 
same and add some 0.1s  to upper range or use another constructor like 
tstzrange(now(),now(),'[]') .

Is there chance to change behavior of storing ranges? Its possible store 
range bounds in internal structure and lower(tstzrange(now(),now())) show 
not NULL value or change default behavior 
tstzrange(timestamptz,timestamptz) - if both args are same, then store as 
'[]', else '[)' and only tstzrange(timestamptz,timestamtz,'[)') and 
tstzrange(timestamptz,timestamtz,'()') store empty range.

It's only suggestion, i don't now if somebody wants store empty range 
without bounds.

We must have some checks to prevent storing empty values on every place 
where can occur this empty range, becouse we don't want lose bound 
information.

Best regards,
-- 
-
Ing. David TUROŇ
LinuxBox.cz, s.r.o.
28. rijna 168, 709 01 Ostrava

tel.:+420 591 166 224
fax:+420 596 621 273
mobil:  +420 732 589 152
www.linuxbox.cz

mobil servis: +420 737 238 656
email servis: ser...@linuxbox.cz
-

Re: remove some STATUS_* symbols

2020-01-16 Thread Kyotaro Horiguchi

At Thu, 16 Jan 2020 14:50:01 +0900, Michael Paquier  wrote 
in 
> On Sat, Jan 11, 2020 at 08:14:17AM +0100, Peter Eisentraut wrote:
> > OK, pushed as it was then.
> 
> Thanks, that looks fine.  I am still not sure whether the second patch
> adding an enum via ProcWaitStatus improves the code readability
> though, so my take would be to discard it for now.  Perhaps others
> think differently, I don't know.

I feel the same about the second patch.

Although actually STATUS_WAITING is used only by ProcSleep and related
functions, likewise STATUS_EOF is seen only in auth.c/h. Other files,
pqcomm.c, crypt.c postmaster.c, hba.c, fe-auth.c , fe-connect.c,
fe-gssapi-common.c are using only STATUS_OK and ERROR. I haven't had a
close look but all of the usages would be equivalent to bool.

On the other hand many functions in fe-*.c and pqcomm.c returns
EOF(-1)/0 instead of STATUS_EOF(-2)/STATUS_OK(0).

We could reorganize the values and their usage but it doesn't seem to
be a big win..

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

Re: [HACKERS] Block level parallel vacuum

2020-01-16 Thread Masahiko Sawada

On Thu, 16 Jan 2020 at 14:11, Amit Kapila  wrote:
>
> On Thu, Jan 16, 2020 at 10:11 AM Mahendra Singh Thalor
>  wrote:
> >
> > On Thu, 16 Jan 2020 at 08:22, Amit Kapila  wrote:
> > >
> > > > 2.
> > > > I checked time taken by vacuum.sql test. Execution time is almost same
> > > > with and without v45 patch.
> > > >
> > > > Without v45 patch:
> > > > Run1) vacuum   ... ok 701 ms
> > > > Run2) vacuum   ... ok 549 ms
> > > > Run3) vacuum   ... ok 559 ms
> > > > Run4) vacuum   ... ok 480 ms
> > > >
> > > > With v45 patch:
> > > > Run1) vacuum   ... ok 842 ms
> > > > Run2) vacuum   ... ok 808 ms
> > > > Run3)  vacuum   ... ok 774 ms
> > > > Run4) vacuum   ... ok 792 ms
> > > >
> > >
> > > I see some variance in results, have you run with autovacuum as off.
> > > I was expecting that this might speed up some cases where parallel
> > > vacuum is used by default.
> >
> > I think, this is expected difference in timing because we are adding
> > some vacuum related test. I am not starting server manually(means I am
> > starting server with only default setting).
> >
>
> Can you once test by setting autovacuum = off?  The autovacuum leads
> to variability in test timing.
>
>

I've also run the regression tests with and without the patch:

* w/o patch and autovacuum = on:  255 ms
* w/o patch and autovacuum = off: 258 ms
* w/ patch and autovacuum = on: 370 ms
* w/ patch and autovacuum = off: 375 ms

> > If we start server with default settings, then we will not hit vacuum
> > related test cases to parallel because size of index relation is very
> > small so we will not do parallel vacuum.

Right. Most indexes (all?) of tables that are used in the regression
tests are smaller than min_parallel_index_scan_size. And we set
min_parallel_index_scan_size to 0 in vacuum.sql but VACUUM would not
be speeded-up much because of the relation size. Since we instead
populate new table for parallel vacuum testing the regression test for
vacuum would take a longer time.

Regards,

-- 
Masahiko Sawadahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Thoughts on "killed tuples" index hint bits support on standby

2020-01-16 Thread Michail Nikolaev

Hello, hackers.

Currently hint bits in the index pages (dead tuples) are set and taken
into account only at primary server. Standby just ignores it. It is
done for reasons, of course (see RelationGetIndexScan and [1]):

* We do this because the xmin on the primary node could easily be
* later than the xmin on the standby node, so that what the primary
* thinks is killed is supposed to be visible on standby. So for correct
* MVCC for queries during recovery we must ignore these hints and check
* all tuples.

Also, according to [2] and cases like [3] it seems to be good idea to
support "ignore_killed_tuples" on standby.

I hope I know the way to support it correctly with reasonable amount of changes.

First thing we need to consider - checksums and wal_log_hints are
widely used these days. So, at any moment master could send FPW page
with new "killed tuples" hints and overwrite hints set by standby.
Moreover it is not possible to distinguish hints are set by primary or standby.

And there is where hot_standby_feedback comes to play. Master node
considers xmin of hot_standy_feedback replicas (RecentGlobalXmin)
while setting "killed tuples" bits. So, if hot_standby_feedback is
enabled on standby for a while - it could safely trust hint bits from
master.
Also, standby could set own hints using xmin it sends to primary
during feedback (but without marking page as dirty).

Of course all is not so easy, there are a few things and corner cases
to care about
* Looks like RecentGlobalXmin could be moved backwards in case of new
replica with lower xmin is connected (or by switching some replica to
hot_standby_feedback=on). We must ensure RecentGlobalXmin is moved
strictly forward.
* hot_standby_feedback could be enabled on the fly. In such a case we
need distinguish transactions which are safe or unsafe to deal with
hints. Standby could receive fresh RecentGlobalXmin as response to
feedback message. All standby transactions with xmin >=
RecentGlobalXmin are safe to use hints.
* hot_standby_feedback could be disabled on the fly. In such situation
standby needs to continue to send feedback while canceling all queries
with ignore_killed_tuples=true. Once all such queries are canceled -
feedback are no longer needed and should be disabled.

Could someone validate my thoughts please? If the idea is mostly
correct - I could try to implement and test it.

[1] -
https://www.postgresql.org/message-id/flat/7067.1529246768%40sss.pgh.pa.us#d9e2e570ba34fc96c4300a362cbe8c38
[2] -
https://www.postgresql.org/message-id/flat/12843.1529331619%40sss.pgh.pa.us#6df9694fdfd5d550fbb38e711d162be8
[3] -
https://www.postgresql.org/message-id/flat/20170428133818.24368.33533%40wrigleys.postgresql.org

1 2 >

1 - 100 of 107 matches

Mail list logo