On 3/27/23 21:42, Ilya Maximets wrote:
> If the schema with no data was read from the clustered storage, it
> should mean a database conversion request. In general, we can get:
>
> 1. Just data --> Transaction record.
> 2. Schema + Data --> Database conversion or raft snapshot install.
> 3. Just schema --> New. Database conversion request.
>
> We cannot distinguish between conversion and snapshot installation
> request in the current implementation, so we will keep handling
> conversion with data in the same way as before, i.e. if data is
> provided, we should use it.
>
> ovsdb-tool is updated to handle this record type as well while
> converting cluster to standalone.
>
> This change doesn't introduce a way for such records to appear in
> the database. That will be added in the future commits targeting
> conversion speed increase.
>
> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
> ---
> ovsdb/ovsdb-server.c | 65 ++++++++++++++++++++++++++++++--------------
> ovsdb/ovsdb-tool.c | 35 ++++++++++++++++++------
> ovsdb/relay.c | 20 +++++++++++---
> ovsdb/relay.h | 7 +++--
> 4 files changed, 91 insertions(+), 36 deletions(-)
>
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 4fea2dbda..91c284e99 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -573,8 +573,9 @@ close_db(struct server_config *config, struct db *db,
> char *comment)
> }
> }
>
> -static void
> -update_schema(struct ovsdb *db, const struct ovsdb_schema *schema, void *aux)
> +static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
> +update_schema(struct ovsdb *db, const struct ovsdb_schema *schema,
> + bool conversion_with_no_data, void *aux)
> {
> struct server_config *config = aux;
>
> @@ -586,13 +587,27 @@ update_schema(struct ovsdb *db, const struct
> ovsdb_schema *schema, void *aux)
> : xasprintf("database %s connected to storage", db->name)));
> }
>
> - ovsdb_replace(db, ovsdb_create(ovsdb_schema_clone(schema), NULL));
> + if (db->schema && conversion_with_no_data) {
> + struct ovsdb *new_db = NULL;
> + struct ovsdb_error *error;
> +
> + error = ovsdb_convert(db, schema, &new_db);
> + if (error) {
> + /* Should never happen, because conversion should have been
> + * checked before writing the schema to the storage. */
> + return error;
> + }
> + ovsdb_replace(db, new_db);
> + } else {
> + ovsdb_replace(db, ovsdb_create(ovsdb_schema_clone(schema), NULL));
> + }
>
> /* Force update to schema in _Server database. */
> struct db *dbp = shash_find_data(config->all_dbs, db->name);
> if (dbp) {
> dbp->row_uuid = UUID_ZERO;
> }
> + return NULL;
> }
>
> static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
> @@ -600,23 +615,30 @@ parse_txn(struct server_config *config, struct db *db,
> const struct ovsdb_schema *schema, const struct json *txn_json,
> const struct uuid *txnid)
> {
> + struct ovsdb_error *error = NULL;
> + struct ovsdb_txn *txn = NULL;
> +
> if (schema) {
> - /* We're replacing the schema (and the data). Destroy the database
> - * (first grabbing its storage), then replace it with the new schema.
> - * The transaction must also include the replacement data.
> + /* We're replacing the schema (and the data). If transaction
> includes
> + * replacement data, destroy the database (first grabbing its
> storage),
> + * then replace it with the new schema. If not, it's a conversion
> + * without data specified. In this case, convert the current
> database
> + * to a new schema instead.
> *
> * Only clustered database schema changes and snapshot installs
> * go through this path.
> */
> - ovs_assert(txn_json);
> ovs_assert(ovsdb_storage_is_clustered(db->db->storage));
>
> - struct ovsdb_error *error = ovsdb_schema_check_for_ephemeral_columns(
> - schema);
> + error = ovsdb_schema_check_for_ephemeral_columns(schema);
> + if (error) {
> + return error;
> + }
> +
> + error = update_schema(db->db, schema, txn_json == NULL, config);
> if (error) {
> return error;
> }
> - update_schema(db->db, schema, config);
> }
>
> if (txn_json) {
> @@ -624,24 +646,25 @@ parse_txn(struct server_config *config, struct db *db,
> return ovsdb_error(NULL, "%s: data without schema",
> db->filename);
> }
>
> - struct ovsdb_txn *txn;
> - struct ovsdb_error *error;
> -
> error = ovsdb_file_txn_from_json(db->db, txn_json, false, &txn);
> - if (!error) {
> - ovsdb_txn_set_txnid(txnid, txn);
> - log_and_free_error(ovsdb_txn_replay_commit(txn));
> - }
> - if (!error && !uuid_is_zero(txnid)) {
> - db->db->prereq = *txnid;
> - }
> if (error) {
> ovsdb_storage_unread(db->db->storage);
> return error;
> }
> + } else if (schema) {
> + /* We just performed conversion without data. Transaction history
> + * was destroyed. Commit a dummy transaction to set the txnid. */
> + txn = ovsdb_txn_create(db->db);
> }
>
> - return NULL;
> + if (txn) {
> + ovsdb_txn_set_txnid(txnid, txn);
> + error = ovsdb_txn_replay_commit(txn);
> + if (!error && !uuid_is_zero(txnid)) {
> + db->db->prereq = *txnid;
> + }
> + }
> + return error;
> }
>
> static void
> diff --git a/ovsdb/ovsdb-tool.c b/ovsdb/ovsdb-tool.c
> index ea2b75b46..e26536532 100644
> --- a/ovsdb/ovsdb-tool.c
> +++ b/ovsdb/ovsdb-tool.c
> @@ -1006,7 +1006,8 @@ raft_header_to_standalone_log(const struct raft_header
> *h,
> }
>
> static void
> -raft_record_to_standalone_log(const struct raft_record *r,
> +raft_record_to_standalone_log(const char *db_file_name,
> + const struct raft_record *r,
> struct ovsdb_log *db_log_data)
> {
> if (r->type == RAFT_REC_ENTRY) {
> @@ -1024,15 +1025,30 @@ raft_record_to_standalone_log(const struct
> raft_record *r,
>
> if (schema_json->type != JSON_NULL) {
> /* This is a database conversion record. Reset the log and
> - * write the new schema. Data JSON should also be part of
> - * the conversion. */
> + * write the new schema. */
> struct ovsdb_schema *schema;
>
> + check_ovsdb_error(ovsdb_schema_from_json(schema_json, &schema));
> +
> if (data_json->type == JSON_NULL) {
> - ovs_fatal(
> - 0, "Invalid database conversion in the log: no data");
> + /* We have a conversion request with no data. There is no
> + * other way as to read back what we have and convert. */
> + struct ovsdb *old_db, *new_db;
> +
> + check_ovsdb_error(ovsdb_log_commit_block(db_log_data));
> +
> + old_db = ovsdb_file_read(db_file_name, false);
> + check_ovsdb_error(ovsdb_convert(old_db, schema, &new_db));
> + ovsdb_destroy(old_db);
> +
> + pa->elems[1] = ovsdb_to_txn_json(
> + new_db, "converted by ovsdb-tool", true);
> + ovsdb_destroy(new_db);
> +
> + json_destroy(data_json);
> + data_json = pa->elems[1];
> }
> - check_ovsdb_error(ovsdb_schema_from_json(schema_json, &schema));
> +
> ovsdb_schema_destroy(schema);
> check_ovsdb_error(ovsdb_log_reset(db_log_data));
> check_ovsdb_error(ovsdb_log_write(db_log_data, schema_json));
> @@ -1654,7 +1670,8 @@ do_compare_versions(struct ovs_cmdl_context *ctx)
> }
>
> static void
> -do_convert_to_standalone(struct ovsdb_log *log, struct ovsdb_log
> *db_log_data)
> +do_convert_to_standalone(const char *db_file_name,
> + struct ovsdb_log *log, struct ovsdb_log
> *db_log_data)
> {
> for (unsigned int i = 0; ; i++) {
> struct json *json;
> @@ -1671,7 +1688,7 @@ do_convert_to_standalone(struct ovsdb_log *log, struct
> ovsdb_log *db_log_data)
> } else {
> struct raft_record r;
> check_ovsdb_error(raft_record_from_json(&r, json));
> - raft_record_to_standalone_log(&r, db_log_data);
> + raft_record_to_standalone_log(db_file_name, &r, db_log_data);
> raft_record_uninit(&r);
> }
> json_destroy(json);
> @@ -1694,7 +1711,7 @@ do_cluster_standalone(struct ovs_cmdl_context *ctx)
> if (strcmp(ovsdb_log_get_magic(log), RAFT_MAGIC) != 0) {
> ovs_fatal(0, "Database is not clustered db.\n");
> }
> - do_convert_to_standalone(log, db_log_data);
> + do_convert_to_standalone(db_file_name, log, db_log_data);
> check_ovsdb_error(ovsdb_log_commit_block(db_log_data));
> ovsdb_log_close(db_log_data);
> ovsdb_log_close(log);
> diff --git a/ovsdb/relay.c b/ovsdb/relay.c
> index 9ff6ed8f3..4a00f0a0a 100644
> --- a/ovsdb/relay.c
> +++ b/ovsdb/relay.c
> @@ -301,6 +301,8 @@ static void
> ovsdb_relay_parse_update(struct relay_ctx *ctx,
> const struct ovsdb_cs_update_event *update)
> {
> + struct ovsdb_error *error = NULL;
> +
> if (!ctx->db) {
> return;
> }
> @@ -308,15 +310,25 @@ ovsdb_relay_parse_update(struct relay_ctx *ctx,
> if (update->monitor_reply && ctx->new_schema) {
> /* There was a schema change. Updating a database with a new schema
> * before processing monitor reply with the new data. */
> - ctx->schema_change_cb(ctx->db, ctx->new_schema,
> - ctx->schema_change_aux);
> + error = ctx->schema_change_cb(ctx->db, ctx->new_schema, false,
> + ctx->schema_change_aux);
> + if (error) {
> + /* Should never happen, but handle this case anyway. */
> + char *s = ovsdb_error_to_string_free(error);
> + VLOG_ERR("%s", s);
Should this be VLOG_ERR_RL?
Regardless:
Acked-by: Dumitru Ceara <dce...@redhat.com>
Thanks!
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
