Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ABI for tracking events #7791

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .unreleased/pr_7791
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implements: #7791 ABI for tracking TimescaleDB events
6 changes: 6 additions & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,12 @@ if(USE_OPENSSL)
endif(MSVC)
endif(USE_OPENSSL)

# This is a kind of strange usage, but it follows the convention of
# PG_VERSION_NUM, where you can get the major version by dividing by 100.
# However, the major number shows up as, e.g., 1500 for PostgreSQL version 15.
math(EXPR TS_VERSION_NUM
"${PROJECT_VERSION_MAJOR} * 10000 + ${PROJECT_VERSION_MINOR}")

configure_file(config.h.in config.h)
add_dependencies(${PROJECT_NAME} gitcheck)
include_directories(${CMAKE_CURRENT_SOURCE_DIR})
Expand Down
13 changes: 13 additions & 0 deletions src/bgw/job.c
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,12 @@ ts_bgw_job_entrypoint(PG_FUNCTION_ARGS)
callbacks->toggle_allocation_blocking && !callbacks->enabled)
callbacks->toggle_allocation_blocking(/*enable=*/true);

TS_PLUGIN_CALLBACK(*timescaledb_plugin_ptr,
bgw_job_starting,
db_oid,
params.job_id,
params.user_oid);

BackgroundWorkerInitializeConnectionByOid(db_oid, params.user_oid, 0);

log_min_messages = ts_guc_bgw_log_level;
Expand Down Expand Up @@ -1287,6 +1293,11 @@ ts_bgw_job_entrypoint(PG_FUNCTION_ARGS)

CommitTransactionCommand();
FlushErrorState();
TS_PLUGIN_CALLBACK(*timescaledb_plugin_ptr,
bgw_job_exiting,
db_oid,
params.job_id,
JOB_FAILURE_IN_EXECUTION);
ReThrowError(edata);
}
PG_END_TRY();
Expand All @@ -1312,6 +1323,8 @@ ts_bgw_job_entrypoint(PG_FUNCTION_ARGS)
INSTR_TIME_SET_CURRENT(duration);
INSTR_TIME_SUBTRACT(duration, start);

TS_PLUGIN_CALLBACK(*timescaledb_plugin_ptr, bgw_job_exiting, db_oid, params.job_id, res);

elog(DEBUG1,
"job %d (%s) exiting with %s: execution time %.2f ms",
params.job_id,
Expand Down
1 change: 1 addition & 0 deletions src/bgw/job.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#pragma once

#include <postgres.h>
#include <fmgr.h>
#include <postmaster/bgworker.h>
#include <storage/lock.h>

Expand Down
1 change: 1 addition & 0 deletions src/config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#define BUILD_OS_VERSION "@CMAKE_SYSTEM_VERSION@"
#define BUILD_PROCESSOR "@CMAKE_SYSTEM_PROCESSOR@"
#define BUILD_POINTER_BYTES @CMAKE_SIZEOF_VOID_P@
#define TS_VERSION_NUM @TS_VERSION_NUM@

/*
* Value should be set in package release scripts. Otherwise
Expand Down
3 changes: 3 additions & 0 deletions src/extension.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "export.h"
#include "extension_constants.h"
#include "timescaledb.h"

extern void ts_extension_invalidate(void);
extern TSDLLEXPORT bool ts_extension_is_loaded(void);
Expand All @@ -21,3 +22,5 @@ extern TSDLLEXPORT char *ts_extension_schema_name(void);
extern const char *ts_experimental_schema_name(void);
extern const char *ts_extension_get_so_name(void);
extern bool ts_extension_is_proxy_table_relid(Oid relid);

extern TimescaleDBPlugin **timescaledb_plugin_ptr;
15 changes: 15 additions & 0 deletions src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@
#include "guc.h"
#include "license_guc.h"
#include "nodes/constraint_aware_append/constraint_aware_append.h"
#include "timescaledb.h"
#include "ts_catalog/catalog.h"
#include "version.h"

#ifdef PG_MODULE_MAGIC
PG_MODULE_MAGIC;
#endif

TimescaleDBPlugin **timescaledb_plugin_ptr = NULL;

extern void _hypertable_cache_init(void);
extern void _hypertable_cache_fini(void);

Expand Down Expand Up @@ -92,6 +95,12 @@ void
_PG_init(void)
{
static bool init_done = false;
static TimescaleDBPlugin timescaledb_plugin_var = {
.size = sizeof(TimescaleDBPlugin),
.magic = PG_MODULE_MAGIC_DATA,
.pg_version = PG_VERSION_NUM,
.ts_version = TS_VERSION_NUM,
};

/*
* Check extension_is loaded to catch certain errors such as calls to
Expand All @@ -107,6 +116,12 @@ _PG_init(void)
if (init_done)
return;

timescaledb_plugin_ptr =
(TimescaleDBPlugin **) find_rendezvous_variable(TIMESCALEDB_PLUGIN_NAME);
/* We're the first, so set up the structure */
if (*timescaledb_plugin_ptr == NULL)
*timescaledb_plugin_ptr = &timescaledb_plugin_var;

_cache_init();
_hypertable_cache_init();
_cache_invalidate_init();
Expand Down
23 changes: 23 additions & 0 deletions src/timescaledb.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* This file and its contents are licensed under the Apache License 2.0.
* Please see the included NOTICE for copyright information and
* LICENSE-APACHE for a copy of the license.
*/
#include <postgres.h>

#include "planner/planner.h"
#include "timescaledb.h"
#include <c.h>

/*
* This file contains function and support for dealing with the ABI. See the
* header file for more information.
*/

/* Structure to go from old definition in mem_guard to the TimescaleDB plugin
structure defined in the header file. */
static TimescaleDBPlugin plugin_callbacks = {
.magic = PG_MODULE_MAGIC_DATA,
.pg_version = PG_VERSION_NUM,
.ts_version = TS_VERSION_NUM,
};
63 changes: 63 additions & 0 deletions src/timescaledb.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* This file and its contents are licensed under the Apache License 2.0.
* Please see the included NOTICE for copyright information and
* LICENSE-APACHE for a copy of the license.
*/

/*
* File with definitions intended for plugins that want to interact with TimescaleDB.
*
* WARNING! Do not add variables, functions, macros, or other stuff that is
* not intended for an external user, e.g., a separate plugin.
*/
#pragma once

#include <postgres.h>
#include <fmgr.h>

/*
* Plugin structure with callbacks.
*
* Other plugins can use this to get information about the running version of
* TimescaleDB as well as callbacks on particular events.
*
* IMPORTANT: If you extend this structure, take care to not remove any fields
* and add new fields last. If you do not do this, you might break plugins
* that are using the wrong version and read some field that you moved or
* deleted.
*/
typedef struct TimescaleDBPlugin
{
size_t size; /* Size of this structure */
Pg_magic_struct magic; /* Pointer to PostgreSQL magic */
int pg_version; /* PostgreSQL version as a number */
int ts_version; /* TimescaleDB version as a number */

/*
* Plugins can fill this in and get callbacks on particular events.
*/
void (*bgw_job_starting)(Oid db_id, int32 job_id, Oid user_oid);
void (*bgw_job_exiting)(Oid db_id, int job_id, int result);
} TimescaleDBPlugin;

#define TIMESCALEDB_PLUGIN_NAME "TimescaleDBPlugin"

/*
* Convenience macro for plugin callback.
*
* The expands to a check if: the plugin pointer is non-zero, the offset of
* the function pointer is inside the structure, and the function pointer is
* non-zero.
*
* If either is false, this is effectively a no-op.
*
* The check that the pointer is inside the size of the structure allows you
* to add new functions to the structure without risking a call to a random
* location if the structure is too "old" for the call.
*/
#define TS_PLUGIN_CALLBACK(PTR, FUNC, ...) \
do \
{ \
if ((PTR) && offsetof(TimescaleDBPlugin, FUNC) < (PTR)->size && (PTR)->FUNC) \
(*(PTR)->FUNC)(__VA_ARGS__); \
} while (0)
Comment on lines +58 to +63
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is prepared to call a function passing some parameters but without returning values. I think we also need a way to cover execute a callback function from other extension returning some value. I.e.: https://github.com/timescale/timescaledb/blob/main/src/tss_callbacks.h#L16

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is prepared to call a function passing some parameters but without returning values.

Yes, I did that since I was unsure if the added functions need a return value. It is easy to add/modify if we need, even in later versions, since this is just a convenience macro.

I think we also need a way to cover execute a callback function from other extension returning some value. I.e.: https://github.com/timescale/timescaledb/blob/main/src/tss_callbacks.h#L16

Ah, good, I was wondering if there were anything else that we should add at this time.

It is possible to have callbacks both ways, but setting it up is a little tricky so thinking about good schemes for dealing with this to reduce the risk of mistakes.

Loading