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.

Sign up for GitHub

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

Jump to bottom

Remove error for correct bootstrap of data node #2505

Merged
merged 1 commit into from Oct 8, 2020
Merged

Remove error for correct bootstrap of data node #2505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 40 additions & 36 deletions tsl/src/data_node.c
View file
Open in desktop
Expand Up @@ -60,7 +60,7 @@ typedef struct DbInfo
NameData collation;
} DbInfo;

static void data_node_validate_database(TSConnection *conn, const DbInfo *database);
static bool data_node_validate_database(TSConnection *conn, const DbInfo *database);

/*
* get_database_info - given a database OID, look up info about the database
Expand Down Expand Up @@ -326,50 +326,52 @@ create_data_node_options(const char *host, int32 port, const char *dbname, const
return list_make4(host_elm, port_elm, dbname_elm, user_elm);
}

/* Returns 'true' if the database was created. */
static bool
data_node_bootstrap_database(TSConnection *conn, const DbInfo *database)
{
const char *const username = PQuser(remote_connection_get_pg_conn(conn));

PGresult *res;

Assert(database);

/* Create the database with the user as owner. This command might fail
* if the database already exists, but we catch the error and continue
* with the bootstrapping if it does. */
res = remote_connection_execf(conn,
"CREATE DATABASE %s ENCODING %s LC_COLLATE %s LC_CTYPE %s "
"TEMPLATE template0 OWNER %s",
quote_identifier(NameStr(database->name)),
quote_identifier(pg_encoding_to_char(database->encoding)),
quote_literal_cstr(NameStr(database->collation)),
quote_literal_cstr(NameStr(database->chartype)),
quote_identifier(username));

if (PQresultStatus(res) != PGRES_COMMAND_OK)
if (data_node_validate_database(conn, database))
{
const char *const sqlstate = PQresultErrorField(res, PG_DIAG_SQLSTATE);
bool database_exists = (sqlstate && strcmp(sqlstate, ERRCODE_DUPLICATE_DATABASE_STR) == 0);

if (!database_exists)
remote_result_elog(res, ERROR);

/* If the database already existed on the remote node, we got a
* duplicate database error above and the database was not created.
*
* In this case, we will log a notice and proceed since it is not an
* error if the database already existed on the remote node. */
/* If the database already existed on the remote node, we will log a
* notice and proceed since it is not an error if the database already
* existed on the remote node. */
elog(NOTICE,
"database \"%s\" already exists on data node, skipping",
NameStr(database->name));
data_node_validate_database(conn, database);
return false;
}

return (PQresultStatus(res) == PGRES_COMMAND_OK);
/* Create the database with the user as owner. There is no need to
* validate the database after this command since it should be created
* correctly. */
PGresult *res =
remote_connection_execf(conn,
"CREATE DATABASE %s ENCODING %s LC_COLLATE %s LC_CTYPE %s "
"TEMPLATE template0 OWNER %s",
quote_identifier(NameStr(database->name)),
quote_identifier(pg_encoding_to_char(database->encoding)),
quote_literal_cstr(NameStr(database->collation)),
quote_literal_cstr(NameStr(database->chartype)),
quote_identifier(username));
if (PQresultStatus(res) != PGRES_COMMAND_OK)
remote_result_elog(res, ERROR);
return true;
}

static void
/* Validate the database.
*
* Errors:
* Will abort with errors if the database exists but is not correctly set
* up.
* Returns:
* true if the database exists and is valid
* false if it does not exist.
*/
static bool
data_node_validate_database(TSConnection *conn, const DbInfo *database)
{
PGresult *res;
Expand All @@ -386,15 +388,16 @@ data_node_validate_database(TSConnection *conn, const DbInfo *database)
ereport(ERROR,
(errcode(ERRCODE_CONNECTION_EXCEPTION), errmsg("%s", PQresultErrorMessage(res))));

/* This only fails if current database is not in pg_database, which would
* very very strange. */
Assert(PQntuples(res) > 0 && PQnfields(res) > 2);
if (PQntuples(res) == 0)
return false;

Assert(PQnfields(res) > 2);
Copy link
Contributor

@pmwkaa pmwkaa Oct 7, 2020 •
edited

Choose a reason for hiding this comment

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

Why not 3? is it for future compatability?

Copy link
Contributor Author

@mkindahl mkindahl Oct 7, 2020 •
edited

Choose a reason for hiding this comment

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

No, this ensure that fields 0, 1, and 2 can be read, which are three elements.


actual_encoding = atoi(PQgetvalue(res, 0, 0));
if (actual_encoding != database->encoding)
ereport(ERROR,
(errcode(ERRCODE_TS_DATA_NODE_INVALID_CONFIG),
errmsg("database encoding mismatch"),
errmsg("database exists but has wrong encoding"),
errdetail("Expected database encoding to be \"%s\" (%u) but it was \"%s\" (%u)",
pg_encoding_to_char(database->encoding),
database->encoding,
Expand All @@ -406,7 +409,7 @@ data_node_validate_database(TSConnection *conn, const DbInfo *database)
if (strcmp(actual_collation, NameStr(database->collation)) != 0)
ereport(ERROR,
(errcode(ERRCODE_TS_DATA_NODE_INVALID_CONFIG),
errmsg("database collation mismatch"),
errmsg("database exists but has wrong collation"),
errdetail("Expected collation \"%s\" but it was \"%s\"",
NameStr(database->collation),
actual_collation)));
Expand All @@ -416,10 +419,11 @@ data_node_validate_database(TSConnection *conn, const DbInfo *database)
if (strcmp(actual_chartype, NameStr(database->chartype)) != 0)
ereport(ERROR,
(errcode(ERRCODE_TS_DATA_NODE_INVALID_CONFIG),
errmsg("database LC_CTYPE mismatch"),
errmsg("database exists but has wrong LC_CTYPE"),
errdetail("Expected LC_CTYPE \"%s\" but it was \"%s\"",
NameStr(database->chartype),
actual_chartype)));
return true;
}

static void
Expand Down
9 changes: 4 additions & 5 deletions tsl/test/expected/data_node_bootstrap.out
View file
Open in desktop
Expand Up @@ -188,8 +188,7 @@ CREATE DATABASE bootstrap_test
\set ON_ERROR_STOP 0
SELECT * FROM add_data_node('bootstrap_test', host => 'localhost',
database => 'bootstrap_test', bootstrap => true);
NOTICE: database "bootstrap_test" already exists on data node, skipping
ERROR: database encoding mismatch
ERROR: database exists but has wrong encoding
\set ON_ERROR_STOP 1
DROP DATABASE bootstrap_test;
----------------------------------------------------------------------
Expand Down Expand Up @@ -224,7 +223,7 @@ SET client_min_messages TO NOTICE;
\set ON_ERROR_STOP 0
SELECT * FROM add_data_node('bootstrap_test', host => 'localhost',
database => 'bootstrap_test', bootstrap => false);
ERROR: database encoding mismatch
ERROR: database exists but has wrong encoding
\set ON_ERROR_STOP 1
DROP DATABASE bootstrap_test;
CREATE DATABASE bootstrap_test
Expand All @@ -242,7 +241,7 @@ SET client_min_messages TO NOTICE;
\set ON_ERROR_STOP 0
SELECT * FROM add_data_node('bootstrap_test', host => 'localhost',
database => 'bootstrap_test', bootstrap => false);
ERROR: database collation mismatch
ERROR: database exists but has wrong collation
\set ON_ERROR_STOP 1
DROP DATABASE bootstrap_test;
CREATE DATABASE bootstrap_test
Expand All @@ -260,7 +259,7 @@ SET client_min_messages TO NOTICE;
\set ON_ERROR_STOP 0
SELECT * FROM add_data_node('bootstrap_test', host => 'localhost',
database => 'bootstrap_test', bootstrap => false);
ERROR: database LC_CTYPE mismatch
ERROR: database exists but has wrong LC_CTYPE
\set ON_ERROR_STOP 1
DROP DATABASE bootstrap_test;
-----------------------------------------------------------------------
Expand Down