Skip to content

Implement partition database discovery via comdb2db.cfg#5254

Merged
chands10 merged 1 commit intobloomberg:mainfrom
chands10:pdb
Sep 10, 2025
Merged

Implement partition database discovery via comdb2db.cfg#5254
chands10 merged 1 commit intobloomberg:mainfrom
chands10:pdb

Conversation

@chands10
Copy link
Contributor

@chands10 chands10 commented Jun 30, 2025

Am taking suggestions for different name to specify sharding in comdb2db.cfg as opposed to just shard

Note: in testsuite will get logmsg get_host_by_name:gethostbyname(%s): errno=0 err=Success because cannot query comdb2db. Code will continue to open shard

CDB2SQLQUERY__Bindvalue__TxtArray *txt =
malloc(sizeof(CDB2SQLQUERY__Bindvalue__TxtArray));
cdb2__sqlquery__bindvalue__txt_array__init(txt);
// txt->elements = (char **)hndl->shards;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like cannot pass char[][] for carray binding text

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 0/0 tests failed ⚠.

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 0/0 tests failed ⚠.

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 0/0 tests failed ⚠.

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 12/615 tests failed ⚠.

The first 10 failing tests are:
disttxn [setup failure]
sc_constraints_logicalsc_generated
sc_redo_logicalsc_generated
partitiondb
remotecreate
cdb2api_unit
unionpar_maxqueue
sc_lotsoftables_logicalsc_generated
sc_lotsoftables
insert_lots_large_tran_generated

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 8/616 tests failed ⚠.

The first 10 failing tests are:
sc_timepart
analyze_fastinit_race
remotecreate
phys_rep_tiered
phys_rep_tiered_firstfile_generated
sc_downgrade
insert_lots_large_tran_generated
cldeadlock

@chands10 chands10 marked this pull request as ready for review July 3, 2025 20:04
Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 8/616 tests failed ⚠.

The first 10 failing tests are:
cldeadlock
updater_latency
analyze_partial_index_off_generated
renametable
sc_downgrade
insert_lots_large_tran_generated
selectv_rcode_disable_svonly_nop_generated

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Error. ⚠.
Smoke testing: Success ✓.
Cbuild submission: Success ✓.
Regression testing: 6/616 tests failed ⚠.

The first 10 failing tests are:
phys_rep_tiered_firstfile_generated
queuedb_rollover
unionpar_maxqueue
sc_lotsoftables_logicalsc_generated
sc_lotsoftables
cldeadlock

tok = strtok_r(NULL, " :,", &last);
(*num_db_hosts)++;
}
} else if (num_shards && strcasecmp("shard", tok) == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The config line details the db names part of a partition, one db name per shard. Lets call it "partition" instead of "shard"/"shards"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to name the variable as partition as well instead of shards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed comdb2db.cfg to be partition instead of shard

debugprint("cdb2_get_dbhosts returns %d\n", rc);
if (rc == 0 && hndl->num_shards > 0) {
char db_shard[DBNAME_LEN];
int shard_num = (hndl->num_shards_sameroom > 0 && (hndl->flags & CDB2_RANDOMROOM)) ? rand() % hndl->num_shards_sameroom : rand() % hndl->num_shards;
Copy link
Contributor

Choose a reason for hiding this comment

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

what happens if the client uses CDB2_ROOM instead of CDB2_RANDOMROOM? do I get a node from same room?

@dorinhogea
Copy link
Contributor

Do you mind adding some comments for the code, it would be much clearer for the future what some more tricky pieces of code are doing?

@chands10 chands10 force-pushed the pdb branch 5 times, most recently from 1b21f8f to 2e935b1 Compare September 9, 2025 20:00
@chands10
Copy link
Contributor Author

chands10 commented Sep 9, 2025

Added some comments to my code! Lmk if there's something confusing where I didn't comment

Signed-off-by: Salil Chandra <schandra107@bloomberg.net>
Copy link
Contributor

@dorinhogea dorinhogea left a comment

Choose a reason for hiding this comment

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

Thank you, looks good to me

Copy link

@roborivers roborivers left a comment

Choose a reason for hiding this comment

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

Coding style check: Success ✓.
Smoke testing: Error ⚠.
Cbuild submission: Success ✓.
Regression testing: 1/627 tests failed ⚠.

The first 10 failing tests are:
logfill_logput_window_generated

@chands10
Copy link
Contributor Author

Thanks Dorin!!

@chands10 chands10 merged commit 4b14f08 into bloomberg:main Sep 10, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants