planner: refactor some code of cross-db binding #58424
Conversation
ti-chi-bot
bot
added
release-note-none
|
Hi @qw4990. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #58424 +/- ##
================================================
+ Coverage 73.5693% 73.6300% +0.0607%
================================================
Files 1680 1681 +1
Lines 463726 463857 +131
================================================
+ Hits 341160 341538 +378
+ Misses 101733 101563 -170
+ Partials 20833 20756 -77
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| type digestBiMapImpl struct { | ||
| mu sync.RWMutex | ||
| noDBDigest2SQLDigest map[string][]string // noDBDigest --> sqlDigests | ||
| sqlDigest2noDBDigest map[string]string // sqlDigest --> noDBDigest |
There was a problem hiding this comment.
sqlDigest2noDBDigest's value can use the unique to reduce memory
There was a problem hiding this comment.
OK I'll update here later, I have a few more refactoring PRs based on this, after merging them all I'll update here.
There was a problem hiding this comment.
unique handle seems interesting
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
| type digestBiMapImpl struct { | ||
| mu sync.RWMutex | ||
| noDBDigest2SQLDigest map[string][]string // noDBDigest --> sqlDigests | ||
| sqlDigest2noDBDigest map[string]string // sqlDigest --> noDBDigest |
There was a problem hiding this comment.
unique handle seems interesting
| } | ||
| } | ||
|
|
||
| func (b *digestBiMapImpl) Add(noDBDigest, sqlDigest string) { |
There was a problem hiding this comment.
exported func should have some comments
| return b.sqlDigest2noDBDigest[sqlDigest] | ||
| } | ||
|
|
||
| func (b *digestBiMapImpl) Copy() digestBiMap { |
There was a problem hiding this comment.
what the case of cp a map here？
There was a problem hiding this comment.
When we load new bindings, we'll copy the old binding cache and then insert new records into it and then replace it with the old one.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AilinKid, hawkingrei The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
What problem does this PR solve?
Issue Number: ref #51347
Problem Summary: planner: refactor some code of cross-db binding
What changed and how does it work?
planner: refactor some code of cross-db binding
NO logical change, just refactor.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.