Skip to content

Commit

Permalink
dbs_idx is used only for writes reordering; a lot of setup for such a…
Browse files Browse the repository at this point in the history
… simple task; simplify it

Signed-off-by: Dorin Hogea <dhogea@bloomberg.net>
  • Loading branch information
dorinhogea committed Jan 17, 2024
1 parent 54b6b97 commit 69e06b7
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 78 deletions.
40 changes: 19 additions & 21 deletions db/comdb2.c
Original file line number Diff line number Diff line change
Expand Up @@ -883,25 +883,15 @@ static inline void _db_hash_del(dbtable *tbl)
hash_del(thedb->sqlalias_hash, tbl);
}

/* lockless -- thedb_lock should be gotten from caller */
int getdbidxbyname_ll(const char *p_name)
{
dbtable *tbl = _db_hash_find(p_name);

return (tbl) ? tbl->dbs_idx : -1;
}

/* get the index offset of table tablename in thedb->dbs array
* notice that since the caller does not hold the lock, accessing
* thedb->dbs[idx] can result in undefined behavior if that table
* is dropped and idx would point to a different table or worse
*/
int get_dbtable_idx_by_name(const char *tablename)
{
Pthread_rwlock_rdlock(&thedb_lock);
int idx = getdbidxbyname_ll(tablename);
Pthread_rwlock_unlock(&thedb_lock);
return idx;
struct dbtable *table = get_dbtable_by_name(tablename);
return table ? table->dbs_idx : -1;
}

dbtable *get_dbtable_by_name(const char *p_name)
Expand Down Expand Up @@ -2326,8 +2316,8 @@ static int llmeta_load_tables(struct dbenv *dbenv, void *tran)
}

struct errstat err = {0};
tbl = create_new_dbtable(dbenv, tblnames[i], csc2text, dbnums[i], i, 0,
0, 0, &err);
tbl = create_new_dbtable(dbenv, tblnames[i], csc2text, dbnums[i], 0, 0,
0, &err);
free(csc2text);
csc2text = NULL;
if (!tbl) {
Expand All @@ -2354,6 +2344,7 @@ static int llmeta_load_tables(struct dbenv *dbenv, void *tran)
*/

/* add to env */
tbl->dbs_idx = i;
dbenv->dbs[i] = tbl;
/* Add table to the hash. */
_db_hash_add(tbl);
Expand Down Expand Up @@ -3317,7 +3308,7 @@ static int init_sqlite_tables(struct dbenv *dbenv)

tbl = create_new_dbtable(dbenv, (char *)sqlite_stats_name[i],
(char *)sqlite_stats_csc2[i], 0,
dbenv->num_dbs, 0, 0, 0, &err);
0, 0, 0, &err);
if (!tbl) {
logmsg(LOGMSG_ERROR, "%s\n", err.errstr);
return -1;
Expand All @@ -3330,6 +3321,7 @@ static int init_sqlite_tables(struct dbenv *dbenv)
/* Add table to thedb->dbs */
dbenv->dbs =
realloc(dbenv->dbs, (dbenv->num_dbs + 1) * sizeof(dbtable *));
tbl->dbs_idx = dbenv->num_dbs;
dbenv->dbs[dbenv->num_dbs++] = tbl;
}
return 0;
Expand Down Expand Up @@ -5792,13 +5784,20 @@ int rename_db(dbtable *db, const char *newname)
return 0;
}

void replace_db_idx(dbtable *p_db, int idx)
/* this is important when we remove a table from array as part of a
* finalize_drop_table and later on (constraint check, other finalize)
* transaction needs to unroll; we reconstructed a "table" to match
* the dropped table and we need to insert it in the previous index
* location
*/
void re_add_dbtable_to_thedb_dbs(dbtable *table)
{
int move = 0;
int idx = table->dbs_idx;
Pthread_rwlock_wrlock(&thedb_lock);

if (idx < 0 || idx >= thedb->num_dbs ||
strcasecmp(thedb->dbs[idx]->tablename, p_db->tablename) != 0) {
strcasecmp(thedb->dbs[idx]->tablename, table->tablename) != 0) {
thedb->dbs =
realloc(thedb->dbs, (thedb->num_dbs + 1) * sizeof(dbtable *));
if (idx < 0 || idx >= thedb->num_dbs) idx = thedb->num_dbs;
Expand All @@ -5811,14 +5810,13 @@ void replace_db_idx(dbtable *p_db, int idx)
thedb->dbs[i]->dbs_idx = i;
}

if (!move) p_db->dbnum = thedb->dbs[idx]->dbnum;
if (!move) table->dbnum = thedb->dbs[idx]->dbnum;

p_db->dbs_idx = idx;
thedb->dbs[idx] = p_db;
thedb->dbs[idx] = table;

/* Add table to the hash. */
if (move == 1) {
_db_hash_add(p_db);
_db_hash_add(table);
}

Pthread_rwlock_unlock(&thedb_lock);
Expand Down
3 changes: 1 addition & 2 deletions db/comdb2.h
Original file line number Diff line number Diff line change
Expand Up @@ -2411,9 +2411,9 @@ struct dbtable *newqdb(struct dbenv *env, const char *name, int avgsz, int pages
int add_queue_to_environment(char *table, int avgitemsz, int pagesize);
void stop_threads(struct dbenv *env);
void resume_threads(struct dbenv *env);
void replace_db_idx(struct dbtable *p_db, int idx);
int add_dbtable_to_thedb_dbs(dbtable *table);
void rem_dbtable_from_thedb_dbs(dbtable *table);
void re_add_dbtable_to_thedb_dbs(dbtable *table);
void hash_sqlalias_db(dbtable *db, const char *newname);
int rename_db(struct dbtable *db, const char *newname);
int ix_find_rnum_by_recnum(struct ireq *iq, int recnum_in, int ixnum,
Expand Down Expand Up @@ -3544,7 +3544,6 @@ int compare_tag_int(struct schema *old, struct schema *new, FILE *out,
int strict);
int cmp_index_int(struct schema *oldix, struct schema *newix, char *descr,
size_t descrlen);
int getdbidxbyname_ll(const char *p_name);
int get_dbtable_idx_by_name(const char *tablename);
int open_temp_db_resume(struct ireq *iq, struct dbtable *db, char *prefix, int resume,
int temp, tran_type *tran);
Expand Down
3 changes: 1 addition & 2 deletions db/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -664,8 +664,7 @@ static int new_table_from_schema(struct dbenv *dbenv, char *tblname,
}

struct errstat err = {0};
db = create_new_dbtable(dbenv, tblname, csc2, dbnum, dbenv->num_dbs, 0, 0,
0, &err);
db = create_new_dbtable(dbenv, tblname, csc2, dbnum, 0, 0, 0, &err);
if (!db) {
logmsg(LOGMSG_ERROR, "%s\ncsc2:\"%s\"\n", err.errstr, csc2);
free(csc2);
Expand Down
16 changes: 6 additions & 10 deletions db/macc_glue.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,15 @@
#include "dynschematypes.h"
#include "dynschemaload.h"

static dbtable *newdb_from_schema(struct dbenv *env, char *tblname, int dbnum,
int dbix);
static dbtable *newdb_from_schema(struct dbenv *env, char *tblname, int dbnum);
static int init_check_constraints(dbtable *tbl);
static int add_cmacc_stmt(dbtable *db, int alt, int allow_ull,
int no_side_effects, struct errstat *err);

struct dbtable *create_new_dbtable(struct dbenv *dbenv, char *tablename,
char *csc2, int dbnum, int indx,
int sc_alt_tablename, int allow_ull,
int no_side_effects, struct errstat *err)
char *csc2, int dbnum, int sc_alt_tablename,
int allow_ull, int no_side_effects,
struct errstat *err)
{
struct dbtable *newtable = NULL;
int rc;
Expand All @@ -47,7 +46,7 @@ struct dbtable *create_new_dbtable(struct dbenv *dbenv, char *tablename,
goto err;
}

newtable = newdb_from_schema(dbenv, tablename, dbnum, indx);
newtable = newdb_from_schema(dbenv, tablename, dbnum);
if (!newtable) {
errstat_set_rcstrf(err, -1, "newdb_from_schema failed for %s",
tablename);
Expand Down Expand Up @@ -111,8 +110,7 @@ int populate_db_with_alt_schema(struct dbenv *dbenv, struct dbtable *db,
return rc;
}

static dbtable *newdb_from_schema(struct dbenv *env, char *tblname, int dbnum,
int dbix)
static dbtable *newdb_from_schema(struct dbenv *env, char *tblname, int dbnum)
{
dbtable *tbl;
int ii;
Expand All @@ -125,8 +123,6 @@ static dbtable *newdb_from_schema(struct dbenv *env, char *tblname, int dbnum,
return NULL;
}

tbl->dbs_idx = dbix;

tbl->dbtype = DBTYPE_TAGGED_TABLE;
tbl->tablename = strdup(tblname);
tbl->dbenv = env;
Expand Down
6 changes: 3 additions & 3 deletions db/macc_glue.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@

/* create a dbtable with the provided schema "csc2" */
struct dbtable *create_new_dbtable(struct dbenv *dbenv, char *tablename,
char *csc2, int dbnum, int indx,
int sc_alt_tablename, int allow_ull,
int no_sideeffects, struct errstat *err);
char *csc2, int dbnum, int sc_alt_tablename,
int allow_ull, int no_sideeffects,
struct errstat *err);

/* populate an existing db with .NEW tags for provided schema "csc2" */
int populate_db_with_alt_schema(struct dbenv *dbenv, struct dbtable *db,
Expand Down
2 changes: 1 addition & 1 deletion db/osqlsqlthr.c
Original file line number Diff line number Diff line change
Expand Up @@ -1790,7 +1790,7 @@ int osql_schemachange_logic(struct schema_change_type *sc,
sc->usedbtablevers = comdb2_table_version(sc->tablename);

if (thd->clnt->dbtran.mode == TRANLEVEL_SOSQL) {
if (usedb && getdbidxbyname_ll(sc->tablename) < 0) {
if (usedb && !get_dbtable_by_name(sc->tablename)) {
unsigned long long version;
char *first_shardname =
timepart_shard_name(sc->tablename, 0, 1, &version);
Expand Down
7 changes: 3 additions & 4 deletions db/tag.c
Original file line number Diff line number Diff line change
Expand Up @@ -6111,7 +6111,7 @@ struct schema *create_version_schema(char *csc2, int version,

ver_db = create_new_dbtable(
thedb, gbl_ver_temp_table, csc2, 0 /* no altname */, 0 /* fake dbnum */,
0 /* fake dbs_idx */, 1 /* allow ull */, 1 /* no side effects */, &err);
1 /* allow ull */, 1 /* no side effects */, &err);
if (!ver_db) {
logmsg(LOGMSG_ERROR, "%s\ncsc2: \"%s\"\n", err.errstr, csc2);
goto done;
Expand Down Expand Up @@ -6185,7 +6185,6 @@ static int load_new_ondisk(dbtable *db, tran_type *tran)
{
int rc;
int bdberr;
int foundix = db->dbs_idx;
int version = get_csc2_version_tran(db->tablename, tran);
int len;
void *old_bdb_handle, *new_bdb_handle;
Expand All @@ -6201,7 +6200,7 @@ static int load_new_ondisk(dbtable *db, tran_type *tran)

struct errstat err = {0};
dbtable *newdb = create_new_dbtable(thedb, db->tablename, csc2, db->dbnum,
foundix, 0, 1, 0, &err);
0, 1, 0, &err);
if (!newdb) {
logmsg(LOGMSG_ERROR, "%s (%s:%d)\n", err.errstr, __FILE__, __LINE__);
goto err;
Expand Down Expand Up @@ -6246,7 +6245,7 @@ static int load_new_ondisk(dbtable *db, tran_type *tran)
bdberr);
db->handle = old_bdb_handle;

replace_db_idx(db, foundix);
re_add_dbtable_to_thedb_dbs(db);
fix_constraint_pointers(db, newdb);

memset(newdb, 0xff, sizeof(dbtable));
Expand Down
4 changes: 2 additions & 2 deletions schemachange/sc_add_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ int add_table_to_environment(char *table, const char *csc2,

struct errstat err = {0};
newdb = create_new_dbtable(thedb, table, (char *)csc2, 0 /*dbnum*/,
thedb->num_dbs, 0 /*no altname*/,
timepartition_name ? 1 : 0 /* allow null if tpt rollout */,
0 /*no altname*/,
timepartition_name ? 1 : 0 /* allow null if tpt rollout */,
0 /* side effects */, &err);
if (!newdb) {
sc_client_error(s, "%s", err.errstr);
Expand Down
9 changes: 1 addition & 8 deletions schemachange/sc_alter_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,6 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s,
int changed;
int i;
char new_prefix[32];
int foundix;
struct scinfo scinfo;
struct errstat err = {0};

Expand Down Expand Up @@ -441,14 +440,8 @@ int do_alter_table(struct ireq *iq, struct schema_change_type *s,
}
Pthread_mutex_lock(&csc2_subsystem_mtx);

/* find which db has a matching name */
if ((foundix = getdbidxbyname_ll(s->tablename)) < 0) {
logmsg(LOGMSG_FATAL, "couldnt find table <%s>\n", s->tablename);
exit(1);
}

newdb = create_new_dbtable(thedb, s->tablename, s->newcsc2, db->dbnum,
foundix, 1 /* sc_alt_name */,
1 /* sc_alt_name */,
(s->same_schema) ? 1 : 0, 0, &err);

if (!newdb) {
Expand Down
9 changes: 1 addition & 8 deletions schemachange/sc_fastinit_table.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ int do_fastinit(struct ireq *iq, struct schema_change_type *s, tran_type *tran)
int bdberr = 0;
int datacopy_odh = 0;
char new_prefix[32];
int foundix;
struct scinfo scinfo;
struct errstat err = {0};

Expand All @@ -81,15 +80,9 @@ int do_fastinit(struct ireq *iq, struct schema_change_type *s, tran_type *tran)

Pthread_mutex_lock(&csc2_subsystem_mtx);

/* find which db has a matching name */
if ((foundix = getdbidxbyname_ll(s->tablename)) < 0) {
logmsg(LOGMSG_FATAL, "couldnt find table <%s>\n", s->tablename);
exit(1);
}

int saved_broken_max_rec_sz = fix_broken_max_rec_sz(s->db->lrl);
newdb = s->newdb =
create_new_dbtable(thedb, s->tablename, s->newcsc2, db->dbnum, foundix,
create_new_dbtable(thedb, s->tablename, s->newcsc2, db->dbnum,
1 /* sc_alt_name */, 1 /* allow ull */, 0, &err);
gbl_broken_max_rec_sz = saved_broken_max_rec_sz;

Expand Down
10 changes: 2 additions & 8 deletions schemachange/sc_struct.c
Original file line number Diff line number Diff line change
Expand Up @@ -1028,15 +1028,9 @@ static int reload_csc2_schema(struct dbtable *db, tran_type *tran,
int changed = 0;
int rc;

int foundix = getdbidxbyname_ll(table);
if (foundix == -1) {
logmsg(LOGMSG_FATAL, "Couldn't find table <%s>\n", table);
exit(1);
}

struct errstat err = {0};
newdb = create_new_dbtable(thedb, table, (char *)csc2, db->dbnum, foundix,
1, 1, 0, &err);
newdb = create_new_dbtable(thedb, table, (char *)csc2, db->dbnum, 1, 1, 0,
&err);

if (newdb == NULL) {
/* shouldn't happen */
Expand Down
11 changes: 2 additions & 9 deletions schemachange/schemachange.c
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ int do_dryrun(struct schema_change_type *s)
}

struct errstat err = {0};
newdb = create_new_dbtable(thedb, s->tablename, s->newcsc2, 0, 0, 1, s->same_schema, 0,
newdb = create_new_dbtable(thedb, s->tablename, s->newcsc2, 0, 1, s->same_schema, 0,
&err);
if (!newdb) {
sc_client_error(s, "%s", err.errstr);
Expand Down Expand Up @@ -937,21 +937,14 @@ static int add_table_for_recovery(struct ireq *iq, struct schema_change_type *s)
}

char new_prefix[32];
int foundix;

if (s->headers != db->odh) {
s->header_change = s->force_dta_rebuild = s->force_blob_rebuild = 1;
}

if ((foundix = getdbidxbyname_ll(s->tablename)) < 0) {
logmsg(LOGMSG_FATAL, "couldnt find table <%s>\n", s->tablename);
abort();
}

struct errstat err = {0};
newdb = create_new_dbtable(thedb, s->tablename, s->newcsc2,
(s->dbnum != -1) ? s->dbnum : 0, foundix, 1, 1,
0, &err);
(s->dbnum != -1) ? s->dbnum : 0, 1, 1, 0, &err);
if (!newdb) {
logmsg(LOGMSG_FATAL, "Shouldn't happen in this piece of code %s:%d.\n",
__FILE__, __LINE__);
Expand Down

0 comments on commit 69e06b7

Please sign in to comment.