Errors and messages are overhauled to conform to the official
PostgreSQL style guide. In particular, the following things from the
guide has been given special attention:
* Correct capitalization of first letter: capitalize only for hints,
and detail messages.
* Correct handling of periods at the end of messages (should be elided
for primary message, but not detail and hint messages).
* The primary message should be short, factual, and avoid reference to
implementation details such as specific function names.
Some messages have also been reworded for clarity and to better
conform with the last bullet above (short primary message). In other
cases, messages have been updated to fix references to, e.g., function
parameters that used the wrong parameter name.
Closes#2364
The sql tests still had version checks and would run EXPLAIN
with different parameters depending on postgres version. Since
all supported postgres versions now support all the parameters
we use we can safely remove the version check.
There was a race condition between the post_restore function
restarting the background worker and the setting of the
restoring flag to "off". If the worker started before the
change to the restoring flag had been committed, it would not
see the change and then die because the worker should exit
when the db is in a restoring state. This modifies the
post_restore function to use a restart instead of a start
so that it waits on the commit to start up. It also adds
logic to the entrypoint to reload config changes caused
by an `ALTER DATABASE SET` command. These changes are
normally only seen at connection startup but we have to
wait until after our lock on the modifying transaction is
released to know whether we should adopt them.
To make tests more stable and to remove some repeated code in the
tests this PR changes the test runner to stop background workers.
Individual tests that need background workers can still start them
and this PR will only stop background workers for the initial database
for the test, behaviour for additional databases created during the
tests will not change.
Change handling of background workers on rename database, the
previous implementation would restart background workers on
RENAME DATABASE unfortunately this leads to a race condition
where its possible that the background workers are restarted
while the RENAME DATABASE has not yet finished leading to the
command not completing successfully. This patch changes the
restart to a stop and generating a warning that background
jobs need to be manually restarted.
This patch also fixes 2 segfaults in the background worker
test code.
These functions improve usability and take all the proper steps to
set restoring on/off (and stop/start background workers in the process)
and to set the license key via a function rather than a guc modification.
CREATE DATABASE WITH TEMPLATE will error if there are still active
sessions to the database used as template database, so we stop
those when executing the command.
ALTER DATABASE SET TABLESPACE will error out if there are still
active sessions to the database so we need to stop
background workers to not prevent the command from succeeding.
When background workers are running RENAME DATABASE will not work
because there are still active sessions to that database open.
This stops background workers on RENAME DATABASE so databases can
be renamed without manually stopping background workers.
Replace hardcoded database name from regression tests with :TEST_DBNAME
Remove creation of database single_2 from test runner and add it to
bgw_launcher and loader test since no other tests used those
use SQL comments in test scripts
Modify the restart action to start schedulers if they do not exist, this fixes a
potential race condition where a scheduler could be started for a given
database, but before it has shut down (because the extension does not exist) a
create extension command is run, the start action then would not change the
state of the worker but it would be waiting on the wrong vxid, so not see that
the extension exists. This also makes it so the start action can be truly
idempotent and not set the vxid on its startup, thereby respecting any restart
action that has taken place before and better defining how each interacts with
the system.
Additionally, we decided that the previous behavior in which launchers were not
started up on, say, alter extension update actions was not all that desirable as
it worked if the stop action had happened, but the database had not restarted,
if the database restarted, then the stop action would have no effect. We decided
that if we desire the ability to disable schedulers for a particular database,
we will implement it in the future as a standalone feature that takes effect
across server restarts rather than having somewhat ill-defined behavior with an
implicit feature of the stop action.
This PR changes the launcher to use a state machine to keep track
of the state of each database scheduler. Further, it add polling
to go through the list of databases and check their states. This
solves several issues
1) A CREATE DATABASE call using a template that already has TimescaleDB
installed previously did not start a scheduler until the next database
restart. A test for this case has been added.
2) A lack of available slots or background workers when a new database was added
meant that the scheduler would not be stared until the next database
restart. Now this will be retried on every polling event.
This PR also simplifies logic since database entries are never removed
from the hash table and thus never added more than once. State
transitions are now easier to read and reason about.
Documentations for the state transitions has been added.
An extension can be dropped indirectly via DROP OWNED. This
PR makes sure that such cases are correctly handled by stopping
the appropriate background workers.
Previously, accounting for BGWs assigned to DB schedulers was incorrect, because NULL scheduler handlers were translating to BGW_STOPPED. This caused the accounter to decrement BGW_counter despite the counter never being incremented. We fix this by imposing the invariant that entries for schedulers should be not be created in the hash table until they have grabbed a slot via the bgw_counter. Also did some cleanup with respect to the message-handling functions. In particular, message_restart_action is now a true-restart rather than a force-restart.
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.
Fix timescaledb.max_background_workers GUC configuration so that there aren't assert errors when higher than max_worker_processes.
Don't error when unable to register background workers due to requesting more workers than max_worker_processes.
Add functions to increment and decrement the bgw_counter multiple times simultaneously.
Error on decrementing background workers below 1 (One worker always reserved for the launcher).
Deal with EXEC_BACKEND/Windows forking differently and causing problems with Shared memory by
a) not mapping multiple times and checking for found on mapping of shared memory segments.
b) dealing separately with reinit and init cases esp in use of GetNamedLWLockTranche
Change behavior of queue reinit and counter reinit to be less intrusive and to process messages might otherwise have been lost.
Remove bgw_launcher test for terminate so the bgw_launcher continues running in the background for further tests.
Slight formatting fixes and unification of error messages.
The launcher controls how Timescale DB schedulers for each database are stopped/started
both at server start time and if they are started or stopped while the server is running
which can happen when, say, an update of the extension is performed.
Includes tests for multiple types of behavior within the launcher, but only a mock for the
db schedulers which will be dealt with in future commits. This launcher code is mostly in the loader,
as such it must remain backwards compatible for the foreseeable future, so significant thought and design
has gone into making interactions with this code well defined and consistent so that maintaining
backwards compatibility is relatively easy.
