From 1c27443ff4e60fc5fd323ed6c63dae3bd2370ccb Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 11 Nov 2019 16:37:27 +0100 Subject: [PATCH 01/25] Stub remote server latency (WIP) --- .../CDB_FederatedServerDiagnostics.sql | 16 +++++++++++++++- test/CDB_FederatedServerDiagnostics.sql | 3 +++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 459338d..49388c5 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -97,6 +97,18 @@ AS $$ $$ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; +-- +-- Get the network latency to a remote PG server +-- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal name) +RETURNS float +AS $$ +BEGIN + RETURN 0.0; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; + -- -- Collect and return diagnostics info from a remote PG into a jsonb @@ -108,11 +120,13 @@ DECLARE remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); remote_postgis_version text := @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal); remote_server_options jsonb := @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal); + remote_server_latency float := @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal); BEGIN RETURN jsonb_build_object( 'server_version', remote_server_version, 'postgis_version', remote_postgis_version, - 'server_options', remote_server_options + 'server_options', remote_server_options, + 'server_latency', remote_server_latency ); END $$ diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index d626004..e48fcb3 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -57,6 +57,9 @@ SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> \echo '%% It returns the remote server options' SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "@@PGPORT@@", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; +\echo '%% It returns the network latency to the remote server' +SELECT '1.7', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_latency": 0.0}'::jsonb; + -- =================================================================== -- Cleanup From dbed2d1edeee1e87478d1b4b1887c078fcae9a9a Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Mon, 11 Nov 2019 18:03:14 +0100 Subject: [PATCH 02/25] More scaffolding for latency in diagnostics --- .../CDB_FederatedServerDiagnostics.sql | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 49388c5..2ba9092 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -97,14 +97,57 @@ AS $$ $$ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; + +-- +-- Get a foreign PG server hostname from the catalog +-- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Host_PG(server_internal name) +RETURNS text +AS $$ +BEGIN + -- TODO: implement + RETURN 'localhost'; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; + + +-- +-- Get a foreign PG server port from the catalog +-- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Port_PG(server_internal name) +RETURNS integer +AS $$ +BEGIN + -- TODO: implement + RETURN 5432; +END +$$ +LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; + +-- +-- Get one measure of network latency to a remote TCP server +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Network_Latency(host text, port integer) +RETURNS float +AS $$ + #--TODO implement + return 0.0 +$$ +LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; + -- -- Get the network latency to a remote PG server -- CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal name) RETURNS float AS $$ +DECLARE + remote_server_host text := @extschema@.__CDB_FS_Foreign_Server_Host_PG(server_internal); + remote_server_port integer := @extschema@.__CDB_FS_Foreign_Server_Port_PG(server_internal); -- TODO: best type for ports + latency float; BEGIN - RETURN 0.0; + latency := @extschema@.__CDB_FS_TCP_Network_Latency(host => remote_server_host, port => remote_server_port); + RETURN latency; END $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; From 8c7a75dd0ccd1a07ccc00a19dd7ab5c862213826 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 12:40:53 +0100 Subject: [PATCH 03/25] Implement __CDB_FS_TCP_Network_Latency --- .../CDB_FederatedServerDiagnostics.sql | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 2ba9092..ca36001 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -126,12 +126,31 @@ $$ LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; -- --- Get one measure of network latency to a remote TCP server -CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Network_Latency(host text, port integer) +-- Get one measure of network latency in ms to a remote TCP server +-- +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Network_Latency( + host text, + port integer DEFAULT 5432, + timeout_seconds float DEFAULT 5.0 +) RETURNS float AS $$ - #--TODO implement - return 0.0 + import socket + from timeit import default_timer as timer + + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + s.settimeout(timeout_seconds) + + t_start = timer() + + try: + s.connect((host, int(port))) + t_stop = timer() + s.shutdown(socket.SHUT_RD) + except (socket.timeout, OSError, socket.error): + return None + + return (t_stop - t_start) * 1000.0 $$ LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; @@ -143,7 +162,7 @@ RETURNS float AS $$ DECLARE remote_server_host text := @extschema@.__CDB_FS_Foreign_Server_Host_PG(server_internal); - remote_server_port integer := @extschema@.__CDB_FS_Foreign_Server_Port_PG(server_internal); -- TODO: best type for ports + remote_server_port integer := @extschema@.__CDB_FS_Foreign_Server_Port_PG(server_internal); latency float; BEGIN latency := @extschema@.__CDB_FS_TCP_Network_Latency(host => remote_server_host, port => remote_server_port); From e14c49b22357ca938310d0eea69e7f4df54a899d Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 12:59:36 +0100 Subject: [PATCH 04/25] Add kind of meaningful test for latency --- test/CDB_FederatedServerDiagnostics.sql | 3 ++- test/CDB_FederatedServerDiagnostics_expect | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index e48fcb3..0043769 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -58,7 +58,8 @@ SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "@@PGPORT@@", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; \echo '%% It returns the network latency to the remote server' -SELECT '1.7', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_latency": 0.0}'::jsonb; +SELECT '1.7', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float > 0.0; +SELECT '1.8', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float < 1000.0; -- =================================================================== diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index 2f1f7ae..d2c543e 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -11,4 +11,7 @@ ERROR: Server "doesNotExist" does not exist 1.5|t %% It returns the remote server options 1.6|t +%% It returns the network latency to the remote server +1.7|t +1.8|t D1| From 43b242e61021bb1af1f5f1ecbca3054ce730106f Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 13:02:04 +0100 Subject: [PATCH 05/25] Retrieve host from the catalog --- scripts-available/CDB_FederatedServerDiagnostics.sql | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index ca36001..b794440 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -104,12 +104,12 @@ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Host_PG(server_internal name) RETURNS text AS $$ -BEGIN - -- TODO: implement - RETURN 'localhost'; -END + SELECT option_value FROM ( + SELECT (pg_options_to_table(srvoptions)).* + FROM pg_foreign_server WHERE srvname = server_internal + ) AS opt WHERE opt.option_name = 'host'; $$ -LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; +LANGUAGE SQL VOLATILE PARALLEL UNSAFE; -- From 26f91001ee117f08c034e5b305d4342075e10a15 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 13:06:14 +0100 Subject: [PATCH 06/25] Retrieve port from the catalog --- scripts-available/CDB_FederatedServerDiagnostics.sql | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index b794440..67398e3 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -118,12 +118,12 @@ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Port_PG(server_internal name) RETURNS integer AS $$ -BEGIN - -- TODO: implement - RETURN 5432; -END + SELECT option_value::integer FROM ( + SELECT (pg_options_to_table(srvoptions)).* + FROM pg_foreign_server WHERE srvname = server_internal + ) AS opt WHERE opt.option_name = 'port'; $$ -LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; +LANGUAGE SQL VOLATILE PARALLEL UNSAFE; -- -- Get one measure of network latency in ms to a remote TCP server From d3ae4b808ced0340f021b2ba6e0cc5bb296dc70d Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 13:27:42 +0100 Subject: [PATCH 07/25] Remove default port 5432 in generic function --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 67398e3..21d3147 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -130,7 +130,7 @@ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; -- CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Network_Latency( host text, - port integer DEFAULT 5432, + port integer, timeout_seconds float DEFAULT 5.0 ) RETURNS float From 9dc547c83855f8e82bc8d88c49bacf6cc55a8663 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 13:37:04 +0100 Subject: [PATCH 08/25] Add test for wrong port --- test/CDB_FederatedServerDiagnostics.sql | 19 +++++++++++++++++-- test/CDB_FederatedServerDiagnostics_expect | 8 ++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index 0043769..47c27bc 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -58,8 +58,22 @@ SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "@@PGPORT@@", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; \echo '%% It returns the network latency to the remote server' -SELECT '1.7', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float > 0.0; -SELECT '1.8', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float < 1000.0; +SELECT '2.1', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float > 0.0; +SELECT '2.2', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float < 1000.0; + +\echo '%% It raises an error if the wrong port is provided' +SELECT 'C2', cartodb.CDB_Federated_Server_Register_PG(server => 'wrong-port'::text, config => '{ + "server": { + "host": "localhost", + "port": "12345" + }, + "credentials": { + "username": "cdb_fs_tester", + "password": "cdb_fs_passwd" + } +}'::jsonb); +SELECT '3.0', cartodb.CDB_Federated_Server_Diagnostics(server => 'wrong-port'); + -- =================================================================== @@ -67,6 +81,7 @@ SELECT '1.8', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->' -- =================================================================== \set QUIET on SELECT 'D1', cartodb.CDB_Federated_Server_Unregister(server => 'loopback'::text); +SELECT 'D2', cartodb.CDB_Federated_Server_Unregister(server => 'wrong-port'::text); -- Reconnect, using a new session in order to close FDW connections \connect DROP DATABASE cdb_fs_tester; diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index d2c543e..e030def 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -12,6 +12,10 @@ ERROR: Server "doesNotExist" does not exist %% It returns the remote server options 1.6|t %% It returns the network latency to the remote server -1.7|t -1.8|t +2.1|t +2.2|t +%% It raises an error if the wrong port is provided +C2| +ERROR: could not connect to server "cdb_fs_wrong-port" D1| +D2| From a032cc08e08ad961e73a23fc990a0e1cbde9000b Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 15:01:56 +0100 Subject: [PATCH 09/25] Measure latency as the avg of n_samples --- scripts-available/CDB_FederatedServerDiagnostics.sql | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 21d3147..5700b0c 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -148,16 +148,19 @@ AS $$ t_stop = timer() s.shutdown(socket.SHUT_RD) except (socket.timeout, OSError, socket.error): + plpy.warning('could not connect to server: %s:%d' % (host, port)) return None - return (t_stop - t_start) * 1000.0 + t_total = (t_stop - t_start) * 1000.0 + plpy.debug('TCP connection %s:%d time=%.2f ms' % (host, port, t_total)) + return t_total $$ LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; -- -- Get the network latency to a remote PG server -- -CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal name) +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal name, n_samples integer DEFAULT 10) RETURNS float AS $$ DECLARE @@ -165,7 +168,10 @@ DECLARE remote_server_port integer := @extschema@.__CDB_FS_Foreign_Server_Port_PG(server_internal); latency float; BEGIN - latency := @extschema@.__CDB_FS_TCP_Network_Latency(host => remote_server_host, port => remote_server_port); + latency := avg(sample) FROM ( + SELECT @extschema@.__CDB_FS_TCP_Network_Latency(host => remote_server_host, port => remote_server_port) sample + FROM generate_series(1, n_samples) + ) q; RETURN latency; END $$ From 18f9f79fe8ae2568fd28309711a1a315c44f0cd4 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 15:08:26 +0100 Subject: [PATCH 10/25] Rename key to server_latency_ms to be self-explanatory --- scripts-available/CDB_FederatedServerDiagnostics.sql | 4 ++-- test/CDB_FederatedServerDiagnostics.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 5700b0c..3a4edf3 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -188,13 +188,13 @@ DECLARE remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); remote_postgis_version text := @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal); remote_server_options jsonb := @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal); - remote_server_latency float := @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal); + remote_server_latency_ms float := @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal); BEGIN RETURN jsonb_build_object( 'server_version', remote_server_version, 'postgis_version', remote_postgis_version, 'server_options', remote_server_options, - 'server_latency', remote_server_latency + 'server_latency_ms', remote_server_latency_ms ); END $$ diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index 47c27bc..1d803d5 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -58,8 +58,8 @@ SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "@@PGPORT@@", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; \echo '%% It returns the network latency to the remote server' -SELECT '2.1', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float > 0.0; -SELECT '2.2', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency')::float < 1000.0; +SELECT '2.1', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::float > 0.0; +SELECT '2.2', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::float < 1000.0; \echo '%% It raises an error if the wrong port is provided' SELECT 'C2', cartodb.CDB_Federated_Server_Register_PG(server => 'wrong-port'::text, config => '{ From 364933b6b983064a1efab6ad6d9c7c735c241583 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Tue, 12 Nov 2019 15:57:15 +0100 Subject: [PATCH 11/25] Compatibility with PG 9.6 and 10 See https://www.postgresql.org/message-id/flat/55137679-fd6b-baf0-bfbf-6558ce5cb409%40postgrespro.ru --- test/CDB_FederatedServerDiagnostics.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index 1d803d5..9e40773 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -58,8 +58,8 @@ SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "@@PGPORT@@", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; \echo '%% It returns the network latency to the remote server' -SELECT '2.1', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::float > 0.0; -SELECT '2.2', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::float < 1000.0; +SELECT '2.1', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::text::float > 0.0; +SELECT '2.2', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::text::float < 1000.0; \echo '%% It raises an error if the wrong port is provided' SELECT 'C2', cartodb.CDB_Federated_Server_Register_PG(server => 'wrong-port'::text, config => '{ From 0362825b8c0ccfed303cdd8f74d4a6238f142f4d Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 11:44:24 +0100 Subject: [PATCH 12/25] Encapsulate python in __CDB_FS_TCP_Foreign_Server_Latency Instead of having a generic function, merge __CDB_FS_TCP_Network_Latency and __CDB_FS_Foreign_Server_Host_PG into one function so that it can only "ping" federated servers. --- .../CDB_FederatedServerDiagnostics.sql | 44 +++++++------------ 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 3a4edf3..8a9188a 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -128,16 +128,24 @@ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; -- -- Get one measure of network latency in ms to a remote TCP server -- -CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Network_Latency( - host text, - port integer, - timeout_seconds float DEFAULT 5.0 +CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Foreign_Server_Latency( + server_internal name, + timeout_seconds integer DEFAULT 5.0, + n_samples integer DEFAULT 10 ) RETURNS float AS $$ import socket from timeit import default_timer as timer + plan = plpy.prepare("SELECT @extschema@.__CDB_FS_Foreign_Server_Host_PG($1) AS host", ['name']) + rv = plpy.execute(plan, [server_internal], 1) + host = rv[0]['host'] + + plan = plpy.prepare("SELECT @extschema@.__CDB_FS_Foreign_Server_Port_PG($1) AS port", ['name']) + rv = plpy.execute(plan, [server_internal], 1) + port = rv[0]['port'] + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.settimeout(timeout_seconds) @@ -157,26 +165,6 @@ AS $$ $$ LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; --- --- Get the network latency to a remote PG server --- -CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal name, n_samples integer DEFAULT 10) -RETURNS float -AS $$ -DECLARE - remote_server_host text := @extschema@.__CDB_FS_Foreign_Server_Host_PG(server_internal); - remote_server_port integer := @extschema@.__CDB_FS_Foreign_Server_Port_PG(server_internal); - latency float; -BEGIN - latency := avg(sample) FROM ( - SELECT @extschema@.__CDB_FS_TCP_Network_Latency(host => remote_server_host, port => remote_server_port) sample - FROM generate_series(1, n_samples) - ) q; - RETURN latency; -END -$$ -LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE; - -- -- Collect and return diagnostics info from a remote PG into a jsonb @@ -185,10 +173,10 @@ CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_Server_Diagnostics_PG(server_int RETURNS jsonb AS $$ DECLARE - remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); - remote_postgis_version text := @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal); - remote_server_options jsonb := @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal); - remote_server_latency_ms float := @extschema@.__CDB_FS_Foreign_Server_Latency_PG(server_internal); + remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); + remote_postgis_version text := @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal); + remote_server_options jsonb := @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal); + remote_server_latency_ms float := @extschema@.__CDB_FS_TCP_Foreign_Server_Latency(server_internal); BEGIN RETURN jsonb_build_object( 'server_version', remote_server_version, From a4a4a1dff13287316b7d6f256065662b8957aa7e Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 12:13:42 +0100 Subject: [PATCH 13/25] Honor n_samples param --- .../CDB_FederatedServerDiagnostics.sql | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 8a9188a..f0205f8 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -130,7 +130,7 @@ LANGUAGE SQL VOLATILE PARALLEL UNSAFE; -- CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Foreign_Server_Latency( server_internal name, - timeout_seconds integer DEFAULT 5.0, + timeout_seconds float DEFAULT 5.0, n_samples integer DEFAULT 10 ) RETURNS float @@ -150,18 +150,22 @@ AS $$ s.settimeout(timeout_seconds) t_start = timer() + samples = [] - try: - s.connect((host, int(port))) - t_stop = timer() - s.shutdown(socket.SHUT_RD) - except (socket.timeout, OSError, socket.error): - plpy.warning('could not connect to server: %s:%d' % (host, port)) - return None + for i in xrange(n_samples): + try: + s.connect((host, int(port))) + t_stop = timer() + s.shutdown(socket.SHUT_RD) + except (socket.timeout, OSError, socket.error), ex: + plpy.warning('could not connect to server %s:%d, %s' % (host, port, str(ex))) + samples.append(None) - t_total = (t_stop - t_start) * 1000.0 - plpy.debug('TCP connection %s:%d time=%.2f ms' % (host, port, t_total)) - return t_total + t_connect = (t_stop - t_start) * 1000.0 + plpy.debug('TCP connection %s:%d time=%.2f ms' % (host, port, t_connect)) + samples.append(t_connect) + + return sum(samples) / n_samples $$ LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; From 980657c4a425a17a22d58b3957f215db23958afc Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 12:16:21 +0100 Subject: [PATCH 14/25] Do not add None to samples in case of connection error Otherwise we get `TypeError: unsupported operand type(s) for +: 'float' and 'NoneType'` when calculating the average. --- scripts-available/CDB_FederatedServerDiagnostics.sql | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index f0205f8..4904a38 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -151,6 +151,7 @@ AS $$ t_start = timer() samples = [] + n_errors = 0 for i in xrange(n_samples): try: @@ -159,13 +160,13 @@ AS $$ s.shutdown(socket.SHUT_RD) except (socket.timeout, OSError, socket.error), ex: plpy.warning('could not connect to server %s:%d, %s' % (host, port, str(ex))) - samples.append(None) + n_errors += 1 t_connect = (t_stop - t_start) * 1000.0 plpy.debug('TCP connection %s:%d time=%.2f ms' % (host, port, t_connect)) samples.append(t_connect) - return sum(samples) / n_samples + return sum(samples) / len(samples) $$ LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; From 5897d8a9cb6b3edbabcf405fa1cebbe866b3643d Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 12:24:59 +0100 Subject: [PATCH 15/25] Fix timer start in measurements --- .../CDB_FederatedServerDiagnostics.sql | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 4904a38..f33d22b 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -146,14 +146,14 @@ AS $$ rv = plpy.execute(plan, [server_internal], 1) port = rv[0]['port'] - s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - s.settimeout(timeout_seconds) - - t_start = timer() - samples = [] - n_errors = 0 - for i in xrange(n_samples): + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + s.settimeout(timeout_seconds) + + t_start = timer() + samples = [] + n_errors = 0 + try: s.connect((host, int(port))) t_stop = timer() From 4b9c048235ff5ea2d95cae26f008a9eb4c28af26 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 12:25:41 +0100 Subject: [PATCH 16/25] Close socket properly before the next iteration This avoid the errors `[Errno 106] Transport endpoint is already connected` which are due to connections kept in TIME_WAIT state. We don't use socket.SO_REUSEADDR as it would make the first connection a special case compared to the following ones. --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index f33d22b..05aac38 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -161,6 +161,8 @@ AS $$ except (socket.timeout, OSError, socket.error), ex: plpy.warning('could not connect to server %s:%d, %s' % (host, port, str(ex))) n_errors += 1 + finally: + s.close() t_connect = (t_stop - t_start) * 1000.0 plpy.debug('TCP connection %s:%d time=%.2f ms' % (host, port, t_connect)) From 19c6de2096fbb63395055d8f1b331eba68f70944 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 14:52:24 +0100 Subject: [PATCH 17/25] Comment out suspected line This may be causing the error ERROR: could not compile PL/Python function "__cdb_fs_tcp_foreign_server_latency" DETAIL: SyntaxError: invalid syntax (, line 26) when testing in PG12 + plpythonu3 --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 05aac38..cfa845e 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -159,7 +159,7 @@ AS $$ t_stop = timer() s.shutdown(socket.SHUT_RD) except (socket.timeout, OSError, socket.error), ex: - plpy.warning('could not connect to server %s:%d, %s' % (host, port, str(ex))) + #-- TODO uncomment & fix: plpy.warning('could not connect to server %s:%d, %s' % (host, port, str(ex))) n_errors += 1 finally: s.close() From 203bd64058d4af05d40200fdc2c7be856c306a67 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 17:02:22 +0100 Subject: [PATCH 18/25] Fix python2/3 compat issue --- scripts-available/CDB_FederatedServerDiagnostics.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index cfa845e..f2df36c 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -158,8 +158,8 @@ AS $$ s.connect((host, int(port))) t_stop = timer() s.shutdown(socket.SHUT_RD) - except (socket.timeout, OSError, socket.error), ex: - #-- TODO uncomment & fix: plpy.warning('could not connect to server %s:%d, %s' % (host, port, str(ex))) + except (socket.timeout, OSError, socket.error) as ex: + plpy.warning('could not connect to server %s:%d, %s' % (host, port, str(ex))) n_errors += 1 finally: s.close() From f7d10adbc25784aa1259e350449c542fe5913b16 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 17:16:53 +0100 Subject: [PATCH 19/25] Fix another python2/3 incompat: xrange --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index f2df36c..29b06ee 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -146,7 +146,7 @@ AS $$ rv = plpy.execute(plan, [server_internal], 1) port = rv[0]['port'] - for i in xrange(n_samples): + for i in range(n_samples): s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.settimeout(timeout_seconds) From 3ae7b1fe055587aceb93b79ffd13a6a203c02eae Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 18:27:14 +0100 Subject: [PATCH 20/25] Fix bug: do not initialize samples in every iter --- scripts-available/CDB_FederatedServerDiagnostics.sql | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 29b06ee..5bd4ab8 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -146,13 +146,14 @@ AS $$ rv = plpy.execute(plan, [server_internal], 1) port = rv[0]['port'] + n_errors = 0 + samples = [] + for i in range(n_samples): s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.settimeout(timeout_seconds) t_start = timer() - samples = [] - n_errors = 0 try: s.connect((host, int(port))) From 57dcfc2a35610d423d146456591e98e436c80c03 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 18:55:09 +0100 Subject: [PATCH 21/25] Add a bunch of statistics --- .../CDB_FederatedServerDiagnostics.sql | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 5bd4ab8..cce5038 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -133,9 +133,11 @@ CREATE OR REPLACE FUNCTION @extschema@.__CDB_FS_TCP_Foreign_Server_Latency( timeout_seconds float DEFAULT 5.0, n_samples integer DEFAULT 10 ) -RETURNS float +RETURNS jsonb AS $$ import socket + import json + import math from timeit import default_timer as timer plan = plpy.prepare("SELECT @extschema@.__CDB_FS_Foreign_Server_Host_PG($1) AS host", ['name']) @@ -169,7 +171,21 @@ AS $$ plpy.debug('TCP connection %s:%d time=%.2f ms' % (host, port, t_connect)) samples.append(t_connect) - return sum(samples) / len(samples) + # --stats + n = len(samples) + mean = sum(samples) / n + var = sum([ (x - mean)**2 for x in samples ]) / (n-1) + stdev = math.sqrt(var) + + ret = { + 'n_samples': n_samples, + 'n_errors': n_errors, + 'avg': round(mean, 3), + 'min': round(min(samples), 3), + 'max': round(max(samples), 3), + 'stdev': round(stdev, 3) + } + return json.dumps(ret) $$ LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; @@ -184,7 +200,7 @@ DECLARE remote_server_version text := @extschema@.__CDB_FS_Foreign_Server_Version_PG(server_internal); remote_postgis_version text := @extschema@.__CDB_FS_Foreign_PostGIS_Version_PG(server_internal); remote_server_options jsonb := @extschema@.__CDB_FS_Foreign_Server_Options_PG(server_internal); - remote_server_latency_ms float := @extschema@.__CDB_FS_TCP_Foreign_Server_Latency(server_internal); + remote_server_latency_ms jsonb := @extschema@.__CDB_FS_TCP_Foreign_Server_Latency(server_internal); BEGIN RETURN jsonb_build_object( 'server_version', remote_server_version, From a340f98b96e0b3c5f19e82962ff4ac581eb0c1a1 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 19:19:39 +0100 Subject: [PATCH 22/25] Update tests with latency stats --- test/CDB_FederatedServerDiagnostics.sql | 22 +++++++++++++++++++--- test/CDB_FederatedServerDiagnostics_expect | 9 ++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index 9e40773..aadb124 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -57,9 +57,25 @@ SELECT '1.5', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> \echo '%% It returns the remote server options' SELECT '1.6', cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback') @> '{"server_options": {"host": "localhost", "port": "@@PGPORT@@", "updatable": "false", "extensions": "postgis", "fetch_size": "1000", "use_remote_estimate": "true"}}'::jsonb; -\echo '%% It returns the network latency to the remote server' -SELECT '2.1', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::text::float > 0.0; -SELECT '2.2', (cartodb.CDB_Federated_Server_Diagnostics(server => 'loopback')->'server_latency_ms')::text::float < 1000.0; +\echo '%% It returns network latency stats to the remote server: min <= avg <= max' +WITH latency AS ( + SELECT CDB_Federated_Server_Diagnostics('loopback')->'server_latency_ms' ms +) SELECT '2.1', (latency.ms->'min')::text::float <= (latency.ms->'avg')::text::float, (latency.ms->'avg')::text::float <= (latency.ms->'max')::text::float +FROM latency; + +\echo '%% Latency stats: 0 <= min <= max <= 1000 ms (local connection)' +WITH latency AS ( + SELECT CDB_Federated_Server_Diagnostics('loopback')->'server_latency_ms' ms +) SELECT '2.2', 0.0 <= (latency.ms->'min')::text::float, (latency.ms->'max')::text::float <= 1000.0 +FROM latency; + +\echo '%% Latency stats: stdev > 0' +WITH latency AS ( + SELECT CDB_Federated_Server_Diagnostics('loopback')->'server_latency_ms' ms +) SELECT '2.3', (latency.ms->'stdev')::text::float >= 0.0 +FROM latency; + + \echo '%% It raises an error if the wrong port is provided' SELECT 'C2', cartodb.CDB_Federated_Server_Register_PG(server => 'wrong-port'::text, config => '{ diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index e030def..df01e73 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -11,9 +11,12 @@ ERROR: Server "doesNotExist" does not exist 1.5|t %% It returns the remote server options 1.6|t -%% It returns the network latency to the remote server -2.1|t -2.2|t +%% It returns network latency stats to the remote server: min <= avg <= max +2.1|t|t +%% Latency stats: 0 <= min <= max <= 1000 ms (local connection) +2.2|t|t +%% Latency stats: stdev > 0 +2.3|t %% It raises an error if the wrong port is provided C2| ERROR: could not connect to server "cdb_fs_wrong-port" From 9b9170448081c075f6fc3f2ce34c5e1fd60a5474 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Wed, 13 Nov 2019 19:27:56 +0100 Subject: [PATCH 23/25] Calculate stats only when they make sense --- .../CDB_FederatedServerDiagnostics.sql | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index cce5038..6c5d369 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -171,21 +171,25 @@ AS $$ plpy.debug('TCP connection %s:%d time=%.2f ms' % (host, port, t_connect)) samples.append(t_connect) - # --stats - n = len(samples) - mean = sum(samples) / n - var = sum([ (x - mean)**2 for x in samples ]) / (n-1) - stdev = math.sqrt(var) - - ret = { + stats = { 'n_samples': n_samples, 'n_errors': n_errors, - 'avg': round(mean, 3), - 'min': round(min(samples), 3), - 'max': round(max(samples), 3), - 'stdev': round(stdev, 3) } - return json.dumps(ret) + n = len(samples) + if n >= 1: + mean = sum(samples) / n + stats.update({ + 'avg': round(mean, 3), + 'min': round(min(samples), 3), + 'max': round(max(samples), 3) + }) + if n >= 2: + var = sum([ (x - mean)**2 for x in samples ]) / (n-1) + stdev = math.sqrt(var) + stats.update({ + 'stdev': round(stdev, 3) + }) + return json.dumps(stats) $$ LANGUAGE plpythonu VOLATILE PARALLEL UNSAFE; From 7f9905cbd5979a2902a61105f4f2257611d53ad1 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Thu, 14 Nov 2019 17:56:29 +0100 Subject: [PATCH 24/25] Test what happens when missing PG port --- test/CDB_FederatedServerDiagnostics.sql | 39 +++++++++++++++++-------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/test/CDB_FederatedServerDiagnostics.sql b/test/CDB_FederatedServerDiagnostics.sql index aadb124..da5e8cb 100644 --- a/test/CDB_FederatedServerDiagnostics.sql +++ b/test/CDB_FederatedServerDiagnostics.sql @@ -23,6 +23,27 @@ SELECT 'C1', cartodb.CDB_Federated_Server_Register_PG(server => 'loopback'::text } }'::jsonb); +SELECT 'C2', cartodb.CDB_Federated_Server_Register_PG(server => 'wrong-port'::text, config => '{ + "server": { + "host": "localhost", + "port": "12345" + }, + "credentials": { + "username": "cdb_fs_tester", + "password": "cdb_fs_passwd" + } +}'::jsonb); + +SELECT 'C3', cartodb.CDB_Federated_Server_Register_PG(server => 'loopback-no-port'::text, config => '{ + "server": { + "host": "localhost" + }, + "credentials": { + "username": "cdb_fs_tester", + "password": "cdb_fs_passwd" + } +}'::jsonb); + \c cdb_fs_tester postgres CREATE EXTENSION postgis; \c contrib_regression postgres @@ -75,21 +96,14 @@ WITH latency AS ( ) SELECT '2.3', (latency.ms->'stdev')::text::float >= 0.0 FROM latency; - - \echo '%% It raises an error if the wrong port is provided' -SELECT 'C2', cartodb.CDB_Federated_Server_Register_PG(server => 'wrong-port'::text, config => '{ - "server": { - "host": "localhost", - "port": "12345" - }, - "credentials": { - "username": "cdb_fs_tester", - "password": "cdb_fs_passwd" - } -}'::jsonb); SELECT '3.0', cartodb.CDB_Federated_Server_Diagnostics(server => 'wrong-port'); +\echo '%% Latency stats: can get them on default PG port 5432 when not provided' +WITH latency AS ( + SELECT CDB_Federated_Server_Diagnostics('loopback-no-port')->'server_latency_ms' ms +) SELECT '2.4', 0.0 <= (latency.ms->'min')::text::float, (latency.ms->'max')::text::float <= 1000.0 +FROM latency; -- =================================================================== @@ -98,6 +112,7 @@ SELECT '3.0', cartodb.CDB_Federated_Server_Diagnostics(server => 'wrong-port'); \set QUIET on SELECT 'D1', cartodb.CDB_Federated_Server_Unregister(server => 'loopback'::text); SELECT 'D2', cartodb.CDB_Federated_Server_Unregister(server => 'wrong-port'::text); +SELECT 'D3', cartodb.CDB_Federated_Server_Unregister(server => 'loopback-no-port'::text); -- Reconnect, using a new session in order to close FDW connections \connect DROP DATABASE cdb_fs_tester; From 064cc2a76bd515eeab00edb8d38f5fe46fecbb31 Mon Sep 17 00:00:00 2001 From: Rafa de la Torre Date: Thu, 14 Nov 2019 17:57:05 +0100 Subject: [PATCH 25/25] Fix for missing port in pg_foreign_server: default to standard 5432 --- scripts-available/CDB_FederatedServerDiagnostics.sql | 2 +- test/CDB_FederatedServerDiagnostics_expect | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/scripts-available/CDB_FederatedServerDiagnostics.sql b/scripts-available/CDB_FederatedServerDiagnostics.sql index 6c5d369..b1e2e4c 100644 --- a/scripts-available/CDB_FederatedServerDiagnostics.sql +++ b/scripts-available/CDB_FederatedServerDiagnostics.sql @@ -146,7 +146,7 @@ AS $$ plan = plpy.prepare("SELECT @extschema@.__CDB_FS_Foreign_Server_Port_PG($1) AS port", ['name']) rv = plpy.execute(plan, [server_internal], 1) - port = rv[0]['port'] + port = rv[0]['port'] or 5432 n_errors = 0 samples = [] diff --git a/test/CDB_FederatedServerDiagnostics_expect b/test/CDB_FederatedServerDiagnostics_expect index df01e73..044ad2f 100644 --- a/test/CDB_FederatedServerDiagnostics_expect +++ b/test/CDB_FederatedServerDiagnostics_expect @@ -1,4 +1,6 @@ C1| +C2| +C3| %% It raises an error if the server does not exist ERROR: Server "doesNotExist" does not exist %% It returns a jsonb object @@ -18,7 +20,9 @@ ERROR: Server "doesNotExist" does not exist %% Latency stats: stdev > 0 2.3|t %% It raises an error if the wrong port is provided -C2| ERROR: could not connect to server "cdb_fs_wrong-port" +%% Latency stats: can get them on default PG port 5432 when not provided +2.4|t|t D1| D2| +D3|