[CONFIG-398] Reflector: query ctlstore_dml_ledger using shard fields (families, tables)
#141
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zhou-hongyu
force-pushed
the
hzhou/CONFIG-398-reflector-query-ctlstore-dml-ledger-using-shard-fields-families-tables
branch
from
ac135f8 to
fa0f995
Compare
## Description Update the `go-sqlite3` dependency for Ctlstore to get to the latest upstream version from https://github.com/mattn/go-sqlite3. Notably we depend on a Segment-specific fork that includes a patch which enables preupdate hooks for our code without requiring a build-time flag to be manually put into place by all users of the ctlstore library. ## Details This depends on segmentio/go-sqlite3#9 to allow the `go get` to succeed. Once that is merged, we deleted the [segment-v1.14.22](https://github.com/segmentio/go-sqlite3/releases/tag/segment-v1.14.22) tag in that repo and recreated it as [v1.14.22-segment](https://github.com/segmentio/go-sqlite3/releases/tag/v1.14.22-segment). > [!TIP] > Despite fixing the `master` of go-sqlite3 to [update module path](https://github.com/segmentio/go-sqlite3/blob/b9c49a6eaf4ca03f2515c9ef9334d20741363b3b/go.mod#L1), I was getting errors doing `go get` after deleting and recreating the `segment-v1.14.22` tag like so: > ``` > % go get github.com/segmentio/[email protected] > go: github.com/segmentio/[email protected] (v1.14.23-0.20240208202956-73b5bef61db6) > requires github.com/segmentio/[email protected]: parsing go.mod: > module declares its path as: github.com/mattn/go-sqlite3 > but was required as: github.com/segmentio/go-sqlite3 > ``` > > @kevinburkesegment explained this was likely due to the Go package cache, so we decided to just delete that previous tag and not try to recreate it. Instead we created a tag with a different name. Testing completed successfully: * [x] ensure CI runs * [x] verify on other services that include ctlstore client, when those services are updated. https://segment.atlassian.net/browse/IO-1695 > [!NOTE] > Replacement for PR #140 which had the wrong version number in its branch name.
zhou-hongyu
force-pushed
the
hzhou/CONFIG-398-reflector-query-ctlstore-dml-ledger-using-shard-fields-families-tables
branch
from
fa0f995 to
519bf36
Compare
zhou-hongyu
changed the title
zhou-hongyu
changed the title
ctlstore_dml_ledger using shard fields (families, tables)
zhou-hongyu
commented
Feb 26, 2024
| } | ||
|
|
||
| if shardingTable != "" { | ||
| whereClause += fmt.Sprintf(" AND CONCAT(family_name,'___',table_name) IN $3") |
There was a problem hiding this comment.
I have consulted Copilot and confirmed that using CONCAT will indeed slow down the query performance:
But I will keep it for now, leaving us two options to optimize:
- As suggested by Copilot, add an additional column
family_table_concatbesidefamily_nameandtable_nameto the ledger table. - Utilize composite key suggested by Erik.
Either way making necessary code changes doesn't feel like a too time-consuming task.
There was a problem hiding this comment.
Or we can change the table column in the ledger table to be the fully-qualified name. Can just change the Executive code to write that instead. Will have to think for a bit whether we actually need the unqualified table name for some reason, but cannot quickly think of why we wouldn't be ok with it.
erikdw
deleted the
hzhou/CONFIG-398-reflector-query-ctlstore-dml-ledger-using-shard-fields-families-tables
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ctlstore_dml_ledgertables usingfamily_name.ctlstore_dml_ledgertables usingtable_name.family_nameexists but notable_namewon't fetch anything(Is it desired?)Testing done by UTs.