Skip to content

Commit ebe0915

Browse files
erimatnorRobAtticus
authored andcommitted
Refactor telemetry and fixes
The installation metadata has been refactored: - The installation metadata store now takes Datums of any type as input and output - Move metadata functions from uuid.c -> metadata.c - Make metadata functions return native types rather than text, including for tests Telemetry tests for ssl and nossl have been combined. Note that PG 9.6 does not have pg_backend_random() that gives us a secure random numbers for UUIDs that we send in telemetry. Therefore, we fall back to the generating the UUID from the timestamp if we are on PG 9.6. This change also fixes a number of test issues. For instance, in the telemetry test the escape char `E` was passed on as part of the response string when set as a variable with `\set`. This was not detected before because the response parser didn't parse the start of the response properly. A number of fixes have been made to the formatting of log messages for telemetry to conform to the PostgreSQL standard as well as being consistent with other messages. Numerous build issues on Windows have been resolved. There is also new functionality to get OS version info on Windows (for telemetry), including a SQL function get_os_info() to retrieve this information. The net library will now allow connecting to a servicename, e.g., http or https. A port is resolved from this service name via getaddrinfo(). An explicit port can still be given, and it that case it will not resolve the service name. Databases the are updated to the new version of the extension will have an install_timestamp in their installation metadata that does not reflect the actual original install date of the extension. To be able to distinguish these updated installations from those that are freshly installed, we add a bogus "epoch" install_timestamp in the update script. Parsing of the version string in the telemetry response has been refactored to be more amenable to testing. Tests have been added.
1 parent faf481b commit ebe0915

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

69 files changed

+1006
-496
lines changed

Diff for: .travis.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ before_install:
1818
- git clone --branch ${PG_GIT_TAG} --depth 1 https://github.com/postgres/postgres.git /tmp/postgres
1919
- docker run -d --name pgbuild -v ${TRAVIS_BUILD_DIR}:/build -v /tmp/postgres:/postgres postgres:${PG_VERSION}-alpine
2020
install:
21-
- docker exec -it pgbuild /bin/sh -c "apk add --no-cache --virtual .build-deps coreutils dpkg-dev gcc libc-dev make util-linux-dev diffutils cmake bison flex curl git && mkdir -p /build/debug"
21+
- docker exec -it pgbuild /bin/sh -c "apk add --no-cache --virtual .build-deps coreutils dpkg-dev gcc libc-dev make util-linux-dev diffutils cmake bison flex curl git openssl-dev && mkdir -p /build/debug"
2222
- docker exec -it pgbuild /bin/sh -c "apk add --no-cache --virtual --update-cache --repository http://dl-3.alpinelinux.org/alpine/edge/testing/ --allow-untrusted lcov"
2323
# We only need to build the regress stuff
2424
- docker exec -it pgbuild /bin/sh -c "cd /postgres && ./configure --enable-coverage --enable-debug --enable-cassert --without-readline --without-zlib && make -C /postgres/src/test/regress"

Diff for: CMakeLists.txt

+9
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ if (NOT CMAKE_BUILD_TYPE)
3434
set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build, options are: None Debug Release RelWithDebInfo MinSizeRel" FORCE)
3535
endif ()
3636

37+
if (CMAKE_BUILD_TYPE MATCHES Debug)
38+
# CMAKE_BUILD_TYPE is set at CMake configuration type. But usage of CMAKE_C_FLAGS_DEBUG is
39+
# determined at build time by running cmake --build . --config Debug (at least on Windows).
40+
# Therefore, we only set these flags if the configuration-time CMAKE_BUILD_TYPE is set to
41+
# Debug. Then Debug enabled builds will only happen on Windows if both the configuration-
42+
# and build-time settings are Debug.
43+
set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -DUSE_ASSERT_CHECKING=1 -DDEBUG=1")
44+
endif (CMAKE_BUILD_TYPE MATCHES Debug)
45+
3746
set(SUPPORTED_COMPILERS "GNU" "Clang" "AppleClang" "MSVC")
3847

3948
# Check for a supported compiler

Diff for: README.md

+7-5
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ See the Releases tab for the latest release.
145145
**Prerequisites**:
146146

147147
- A standard [PostgreSQL 9.6 or 10 64-bit installation](https://www.enterprisedb.com/downloads/postgres-postgresql-downloads#windows)
148+
- OpenSSL for Windows
148149
- Microsoft Visual Studio 2017 with CMake and Git components
149150
- OR Visual Studio 2015/2016 with [CMake](https://cmake.org/) version 3.4 or greater and Git
150151
- Make sure all relevant binaries are in your PATH: `pg_config`, `cmake`, `MSBuild`
@@ -163,16 +164,17 @@ cd timescaledb
163164
git checkout 0.8.0
164165

165166
# Bootstrap the build system
166-
./bootstrap.bat
167+
bootstrap.bat
167168

168169
# To build the extension from command line
169-
cd build
170-
MSBuild.exe timescaledb.sln
170+
cmake --build ./build --config Release
171171

172172
# To install
173-
MSBuild.exe /p:Configuration=Release INSTALL.vcxproj
173+
cmake --build ./build --config Release --target install
174174

175-
# Alternatively, open build/timescaledb.sln in Visual Studio and build
175+
176+
# Alternatively, build in Visual Studio via its built-in support for CMake or by
177+
# opening the generated build/timescaledb.sln solution file.
176178
```
177179

178180
### Additional documentation

Diff for: bootstrap.bat

+1-2
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,4 @@ cmake %SRC_DIR% -A x64 %*
2929
ECHO ---
3030
ECHO TimescaleDB build system initialized in %BUILD_DIR%.
3131
ECHO To compile, do:
32-
ECHO cd %BUILD_DIR%
33-
ECHO MSBuild.exe timescaledb.sln
32+
ECHO cmake --build %BUILD_DIR% --config Release

Diff for: scripts/docker-build.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ else
2525
docker run -d --name ${BUILD_CONTAINER_NAME} -v ${BASE_DIR}:/src postgres:${PG_IMAGE_TAG}
2626

2727
# Install build dependencies
28-
docker exec -u root -it ${BUILD_CONTAINER_NAME} /bin/bash -c "apk add --no-cache --virtual .build-deps gdb git coreutils dpkg-dev gcc libc-dev make cmake util-linux-dev diffutils && mkdir -p /build/debug"
28+
docker exec -u root -it ${BUILD_CONTAINER_NAME} /bin/bash -c "apk add --no-cache --virtual .build-deps gdb git coreutils dpkg-dev gcc libc-dev make cmake util-linux-dev diffutils openssl-dev && mkdir -p /build/debug"
2929

3030
docker commit -a $USER -m "TimescaleDB build base image version $PG_IMAGE_TAG" ${BUILD_CONTAINER_NAME} ${BUILD_IMAGE_NAME}:${PG_IMAGE_TAG}
3131
fi

Diff for: sql/CMakeLists.txt

+1-4
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,9 @@ function(cat_files SRC_FILE_LIST OUTPUT_FILE)
107107
if (WIN32)
108108
# Make list of files into string of files separated by "+"
109109
# to make Windows copy concatenate them
110-
111110
file(TO_NATIVE_PATH "${SRC_FILE_LIST}" SRC_FILE_LIST_NATIVE)
112-
string(REPLACE ";" ";+" SQL_LIST_JOINED "${SRC_FILE_LIST_NATIVE}")
113-
111+
string(REPLACE ";" ";+;" SQL_LIST_JOINED "${SRC_FILE_LIST_NATIVE}")
114112
file(TO_NATIVE_PATH "${OUTPUT_FILE}" OUTPUT_FILE_NATIVE)
115-
116113
set(CAT_CMD copy /B /y ${SQL_LIST_JOINED} "\"${OUTPUT_FILE_NATIVE}\"" >NUL)
117114
else ()
118115
set(CAT_CMD cat ${SRC_FILE_LIST} > ${OUTPUT_FILE})

Diff for: sql/installation_metadata.sql

+10-7
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
-- Insert uuid and install_timestamp on database creation.
2-
-- Don't create exported_uuid because this shouldn't get called on a create
3-
CREATE FUNCTION _timescaledb_internal.generate_uuid_external() RETURNS TEXT
4-
AS '@MODULE_PATHNAME@', 'generate_uuid_external' LANGUAGE C VOLATILE STRICT;
5-
INSERT INTO _timescaledb_catalog.installation_metadata SELECT 'uuid', _timescaledb_internal.generate_uuid_external();
6-
INSERT INTO _timescaledb_catalog.installation_metadata SELECT 'install_timestamp', now();
7-
DROP FUNCTION _timescaledb_internal.generate_uuid_external();
1+
CREATE OR REPLACE FUNCTION _timescaledb_internal.generate_uuid() RETURNS UUID
2+
AS '@MODULE_PATHNAME@', 'ts_uuid_generate' LANGUAGE C VOLATILE STRICT;
3+
4+
-- Insert uuid and install_timestamp on database creation. Don't
5+
-- create exported_uuid because it gets exported and installed during
6+
-- pg_dump, which would cause a conflict.
7+
INSERT INTO _timescaledb_catalog.installation_metadata
8+
SELECT 'uuid', _timescaledb_internal.generate_uuid() ON CONFLICT DO NOTHING;
9+
INSERT INTO _timescaledb_catalog.installation_metadata
10+
SELECT 'install_timestamp', now() ON CONFLICT DO NOTHING;

Diff for: sql/pre_install/tables.sql

+1-2
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,7 @@ CREATE TABLE IF NOT EXISTS _timescaledb_catalog.installation_metadata (
180180
key NAME NOT NULL PRIMARY KEY,
181181
value TEXT NOT NULL
182182
);
183-
SELECT
184-
pg_catalog.pg_extension_config_dump('_timescaledb_catalog.installation_metadata', $$WHERE key='exported_uuid'$$);
183+
SELECT pg_catalog.pg_extension_config_dump('_timescaledb_catalog.installation_metadata', $$WHERE key='exported_uuid'$$);
185184

186185
-- Set table permissions
187186
GRANT SELECT ON ALL TABLES IN SCHEMA _timescaledb_catalog TO PUBLIC;

Diff for: sql/updates/0.11.0--0.11.1-dev.sql

+8-5
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,11 @@ BEGIN
5454
users. For more information and how to disable, please see our docs https://docs.timescaledb.com/using-timescaledb/telemetry.\n';
5555
END;
5656
$$;
57-
CREATE FUNCTION _timescaledb_internal.generate_uuid_external() RETURNS TEXT
58-
AS '@MODULE_PATHNAME@', 'generate_uuid_external' LANGUAGE C VOLATILE STRICT;
59-
INSERT INTO _timescaledb_catalog.installation_metadata SELECT 'uuid', _timescaledb_internal.generate_uuid_external();
60-
INSERT INTO _timescaledb_catalog.installation_metadata SELECT 'install_timestamp', GetCurrentTimestamp();
61-
DROP FUNCTION _timescaledb_internal.generate_uuid_external();
57+
58+
CREATE TABLE IF NOT EXISTS _timescaledb_catalog.installation_metadata (
59+
key NAME NOT NULL PRIMARY KEY,
60+
value TEXT NOT NULL
61+
);
62+
SELECT pg_catalog.pg_extension_config_dump('_timescaledb_catalog.installation_metadata', $$WHERE key='exported_uuid'$$);
63+
64+
INSERT INTO _timescaledb_catalog.installation_metadata SELECT 'install_timestamp', to_timestamp(0);

Diff for: sql/version.sql

+3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
CREATE OR REPLACE FUNCTION _timescaledb_internal.get_git_commit() RETURNS TEXT
22
AS '@MODULE_PATHNAME@', 'get_git_commit' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
33

4+
CREATE OR REPLACE FUNCTION _timescaledb_internal.get_os_info() RETURNS TABLE(sysname TEXT, version TEXT, release TEXT)
5+
AS '@MODULE_PATHNAME@', 'ts_get_os_info' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;
6+
47
CREATE OR REPLACE FUNCTION get_report() RETURNS TEXT
58
AS '@MODULE_PATHNAME@', 'ts_get_telemetry_report' LANGUAGE C IMMUTABLE STRICT PARALLEL SAFE;

Diff for: src/CMakeLists.txt

+11-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
set(CMAKE_C_FLAGS_DEBUG "-DUSE_ASSERT_CHECKING=1 -DDEBUG=1")
2-
31
# Check if PostgreSQL has OpenSSL enabled
42
file(STRINGS ${PG_INCLUDEDIR}/pg_config.h PG_USE_OPENSSL REGEX "#define USE_OPENSSL [01]")
53
string(REGEX REPLACE "#define USE_OPENSSL ([01])" "\\1" USE_OPENSSL ${PG_USE_OPENSSL})
@@ -41,7 +39,7 @@ else ()
4139
endif ()
4240

4341
if (WIN32)
44-
set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${PG_LIBDIR}/postgres.lib ws2_32.lib")
42+
set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} ${PG_LIBDIR}/postgres.lib ws2_32.lib Version.lib")
4543
set(CMAKE_C_FLAGS "-D_CRT_SECURE_NO_WARNINGS")
4644
include_directories(${PG_INCLUDEDIR_SERVER}/port/win32)
4745

@@ -94,7 +92,8 @@ set(HEADERS
9492
subspace_store.h
9593
tablespace.h
9694
trigger.h
97-
utils.h)
95+
utils.h
96+
version.h)
9897

9998
set(SOURCES
10099
agg_bookend.c
@@ -142,7 +141,6 @@ set(SOURCES
142141
utils.c
143142
version.c)
144143

145-
configure_file(version.h.in version.h)
146144
set(GITCOMMIT_H ${CMAKE_CURRENT_BINARY_DIR}/gitcommit.h)
147145

148146
# Add test source code in Debug builds
@@ -168,22 +166,18 @@ else ()
168166
VERBATIM)
169167
endif (WIN32)
170168

171-
add_library(${PROJECT_NAME} MODULE ${SOURCES} ${TEST_SOURCES} ${HEADERS} ${GITCOMMIT_H})
169+
add_library(${PROJECT_NAME} MODULE ${SOURCES} ${TEST_SOURCES} ${GITCOMMIT_H})
172170

173171
if (OPENSSL_FOUND AND USE_OPENSSL)
174172
target_link_libraries(${PROJECT_NAME} ${OPENSSL_LIBRARIES})
175173
endif (OPENSSL_FOUND AND USE_OPENSSL)
176174

177-
include_directories(${CMAKE_CURRENT_SOURCE_DIR})
178-
add_subdirectory(bgw)
179-
add_subdirectory(net)
180-
add_subdirectory(telemetry)
181-
182175
if (OPENSSL_FOUND AND USE_OPENSSL)
183176
target_link_libraries(${PROJECT_NAME} ${OPENSSL_LIBRARIES})
184177
endif (OPENSSL_FOUND AND USE_OPENSSL)
185178

186179
if(CMAKE_BUILD_TYPE MATCHES Debug)
180+
set(DEBUG 1)
187181
add_subdirectory(../test/src "${CMAKE_CURRENT_BINARY_DIR}/test/src")
188182
endif(CMAKE_BUILD_TYPE MATCHES Debug)
189183

@@ -200,4 +194,10 @@ install(
200194
TARGETS ${PROJECT_NAME}
201195
DESTINATION ${PG_PKGLIBDIR})
202196

197+
configure_file(config.h.in config.h)
198+
199+
include_directories(${CMAKE_CURRENT_SOURCE_DIR})
200+
add_subdirectory(bgw)
201+
add_subdirectory(net)
202+
add_subdirectory(telemetry)
203203
add_subdirectory(loader)

Diff for: src/catalog.h

+6-18
Original file line numberDiff line numberDiff line change
@@ -589,9 +589,6 @@ enum Anum_bgw_job_stat_pkey_idx
589589
******************************/
590590

591591
#define INSTALLATION_METADATA_TABLE_NAME "installation_metadata"
592-
#define INSTALLATION_METADATA_UUID_KEY_NAME "uuid"
593-
#define INSTALLATION_METADATA_EXPORTED_UUID_KEY_NAME "exported_uuid"
594-
#define INSTALLATION_METADATA_TIMESTAMP_KEY_NAME "install_timestamp"
595592

596593
enum Anum_installation_metadata
597594
{
@@ -627,20 +624,11 @@ enum
627624
_MAX_INSTALLATION_METADATA_INDEX,
628625
};
629626

630-
#define MAX(a, b) \
631-
((long)(a) > (long)(b) ? (a) : (b))
632-
633-
#define _MAX_TABLE_INDEXES \
634-
MAX(_MAX_HYPERTABLE_INDEX, \
635-
MAX(_MAX_DIMENSION_INDEX, \
636-
MAX(_MAX_DIMENSION_SLICE_INDEX, \
637-
MAX(_MAX_CHUNK_CONSTRAINT_INDEX, \
638-
MAX(_MAX_CHUNK_INDEX_INDEX, \
639-
MAX(_MAX_TABLESPACE_INDEX, \
640-
MAX(_MAX_BGW_JOB_INDEX, \
641-
MAX(_MAX_BGW_JOB_STAT_INDEX, \
642-
MAX(_MAX_INSTALLATION_METADATA_INDEX, \
643-
_MAX_CHUNK_INDEX)))))))))
627+
/*
628+
* The maximum number of indexes a catalog table can have.
629+
* This needs to be bumped in case of new catalog tables that have more indexes.
630+
*/
631+
#define _MAX_TABLE_INDEXES 5
644632

645633
typedef enum CacheType
646634
{
@@ -659,8 +647,8 @@ typedef struct Catalog
659647
const char *schema_name;
660648
const char *name;
661649
Oid id;
662-
Oid index_ids[_MAX_TABLE_INDEXES];
663650
Oid serial_relid;
651+
Oid index_ids[_MAX_TABLE_INDEXES];
664652
} tables[_MAX_CATALOG_TABLES];
665653

666654
Oid cache_schema_id;

Diff for: src/compat.h

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#ifndef TIMESCALEDB_COMPAT_H
22
#define TIMESCALEDB_COMPAT_H
33

4+
#include <postgres.h>
5+
46
#include "export.h"
57

68
#define is_supported_pg_version_96(version) ((version >= 90603) && (version < 100000))

Diff for: src/version.h.in renamed to src/config.h.in

+25-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
#ifndef TIMESCALEDB_VERSION_H
2-
#define TIMESCALEDB_VERSION_H
1+
#ifndef TIMESCALEDB_CONFIG_H
2+
#define TIMESCALEDB_CONFIG_H
33

44
#define TIMESCALEDB_VERSION "@PROJECT_VERSION@"
55
#define TIMESCALEDB_VERSION_MOD "@PROJECT_VERSION_MOD@"
@@ -10,8 +10,28 @@
1010
#define BUILD_OS_NAME "@CMAKE_SYSTEM_NAME@"
1111
#define BUILD_OS_VERSION "@CMAKE_SYSTEM_VERSION@"
1212
#define PG_VERSION "@PG_VERSION@"
13-
// Value should be set in package release scripts. Otherwise
14-
// defaults to "source"
13+
/*
14+
* Value should be set in package release scripts. Otherwise
15+
* defaults to "source"
16+
*/
1517
#define TIMESCALEDB_INSTALL_METHOD "@PROJECT_INSTALL_METHOD@"
1618

17-
#endif /* TIMESCALEDB_VERSION_H */
19+
/* Platform */
20+
#ifndef WIN32
21+
#cmakedefine WIN32
22+
#endif
23+
#ifndef MSVC
24+
#cmakedefine MSVC
25+
#endif
26+
#ifndef UNIX
27+
#cmakedefine UNIX
28+
#endif
29+
#ifndef APPLE
30+
#cmakedefine APPLE
31+
#endif
32+
33+
#ifndef DEBUG
34+
#cmakedefine DEBUG
35+
#endif
36+
37+
#endif /* TIMESCALEDB_CONFIG_H */

Diff for: src/export.h

+2
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
#include <postgres.h>
55

6+
#include "config.h"
7+
68
/* Definitions for symbol exports */
79

810
#define TS_CAT(x,y) x ## y

Diff for: src/extension.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
#include "catalog.h"
2222
#include "extension.h"
2323
#include "guc.h"
24-
#include "version.h"
24+
#include "config.h"
2525
#include "extension_utils.c"
2626
#include "compat.h"
2727

Diff for: src/guc.c

+15-3
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,23 @@
44

55
#include "guc.h"
66
#include "hypertable_cache.h"
7+
#include "telemetry/telemetry.h"
8+
9+
typedef enum TelemetryLevel
10+
{
11+
TELEMETRY_OFF,
12+
TELEMETRY_BASIC,
13+
} TelemetryLevel;
14+
15+
/* Define which level means on. We use this object to have at least one object
16+
* of type TelemetryLevel in the code, otherwise pgindent won't work for the
17+
* type */
18+
static const TelemetryLevel on_level = TELEMETRY_BASIC;
719

820
bool
921
telemetry_on()
1022
{
11-
return guc_telemetry_level == TELEMETRY_BASIC;
23+
return guc_telemetry_level == on_level;
1224
}
1325

1426
static const struct config_enum_entry telemetry_level_options[] =
@@ -24,7 +36,7 @@ bool guc_restoring = false;
2436
bool guc_constraint_aware_append = true;
2537
int guc_max_open_chunks_per_insert = 10;
2638
int guc_max_cached_chunks_per_hypertable = 10;
27-
char *guc_telemetry_endpoint = "https://telemetry.timescale.com";
39+
char *guc_telemetry_endpoint = TELEMETRY_ENDPOINT;
2840
int guc_telemetry_level = TELEMETRY_BASIC;
2941

3042
static void
@@ -113,7 +125,7 @@ _guc_init(void)
113125
"URI for telemetry endpoint",
114126
"URI for telemetry endpoint",
115127
&guc_telemetry_endpoint,
116-
"https://telemetry.timescale.com",
128+
guc_telemetry_endpoint,
117129
PGC_INTERNAL,
118130
0,
119131
NULL,

Diff for: src/guc.h

+1-6
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
11
#ifndef TIMESCALEDB_GUC_H
22
#define TIMESCALEDB_GUC_H
3-
#include <postgres.h>
43

5-
typedef enum TelemetryLevel
6-
{
7-
TELEMETRY_OFF,
8-
TELEMETRY_BASIC,
9-
} TelemetryLevel;
4+
#include <postgres.h>
105

116
extern bool telemetry_on(void);
127

0 commit comments

Comments
 (0)