Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix : check if table meta cache should be refreshed in AT mode #4734

Merged
merged 61 commits into from
Feb 28, 2023

Conversation

Bughue
Copy link
Contributor

@Bughue Bughue commented Jun 30, 2022

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

AT模式下,在构造record的时候会遍历resultset并从tablemeta里获取表结构。如果数据库表中新建了一个字段,就会报空指针。
at io.seata.rm.datasource.sql.struct.TableRecords.buildRecords(TableRecords.java:195)

旧方案(已废弃):

  • 先把原本的buildRecords改为private,加上检测,在检测到null时抛出一个固定的TableMetaException异常
  • 对外提供新的buildRecords方法,要求传入StatementProxy,方便在遇到TableMetaException直接刷新meta
  • 新方法在捕捉到异常后会进行tablemeta刷新(靠StatementProxy提供connection、dbtype、resourceid)
  • 现在用双锁来控制并发,内存标识在tableMeta里面(保证同key只会刷新一次,不管是不是并发)
  • AbstractUndoExecutor里没有StatementProxy,但undo过程如果meta都不对应的话肯定是回滚不了,已忽略

新方案(已实现):

  • 把原本的buildRecords加上检测,在检测到null时抛出一个固定的TableMetaException异常
  • AbstractDMLBaseExecutor.executeAutoCommitFalse在捕捉到异常后会提出一个meta刷新事件(加入队列)
  • DataSourceProxy在初始化的时候就建立一个阻塞队列和一个专门处理meta刷新的单一线程的线程池,他会从队列不断取出刷新事件进行刷新(如果1秒前已经刷过就跳过)

关于单测:在TableRecordsTest模拟了字段不同的情况

Ⅱ. Does this pull request fix one issue?

fixes #4572
fixes #5292

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@Bughue Bughue changed the title [WIP] bugfix : check if table meta cache should be refreshed in AT mode bugfix : check if table meta cache should be refreshed in AT mode Jun 30, 2022
@l81893521 l81893521 self-requested a review July 1, 2022 07:45
@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2022

Codecov Report

Merging #4734 (16ca49a) into develop (00ca98c) will decrease coverage by 0.07%.
The diff coverage is 56.89%.

❗ Current head 16ca49a differs from pull request most recent head 96bae53. Consider uploading reports for the commit 96bae53 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #4734      +/-   ##
=============================================
- Coverage      48.63%   48.57%   -0.07%     
+ Complexity      4184     4180       -4     
=============================================
  Files            743      744       +1     
  Lines          26646    26688      +42     
  Branches        3328     3332       +4     
=============================================
+ Hits           12960    12963       +3     
- Misses         12296    12326      +30     
- Partials        1390     1399       +9     
Impacted Files Coverage Δ
...datasource/exec/mysql/MySQLUpdateJoinExecutor.java 78.34% <ø> (ø)
...n/java/io/seata/rm/datasource/DataSourceProxy.java 76.59% <33.33%> (-3.80%) ⬇️
...ta/rm/datasource/exec/AbstractDMLBaseExecutor.java 51.85% <50.00%> (-5.30%) ⬇️
...m/datasource/sql/struct/TableMetaCacheFactory.java 52.63% <50.00%> (-22.37%) ⬇️
...ta/rm/datasource/exception/TableMetaException.java 66.66% <80.00%> (ø)
...o/seata/rm/datasource/sql/struct/TableRecords.java 68.46% <100.00%> (+1.48%) ⬆️
...n/src/main/java/io/seata/common/util/IdWorker.java 77.08% <0.00%> (-6.25%) ⬇️
...ource/sql/struct/cache/AbstractTableMetaCache.java 82.35% <0.00%> (-2.95%) ⬇️
...rage/redis/store/RedisTransactionStoreManager.java 74.93% <0.00%> (-1.57%) ⬇️
...torage/file/store/FileTransactionStoreManager.java 55.62% <0.00%> (-0.65%) ⬇️
... and 1 more

@CLAassistant
Copy link

CLAassistant commented Dec 12, 2022

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@funky-eyes funky-eyes added this to the 1.7.0 milestone Feb 3, 2023
TableMetaRefreshHolder(DataSourceProxy dataSource) {
this.dataSource = dataSource;
this.lastRefreshFinishTime = System.currentTimeMillis() - TABLE_META_REFRESH_INTERVAL_TIME;
this.tableMetaRefreshQueue = new LinkedBlockingQueue<>();
Copy link
Member

Choose a reason for hiding this comment

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

limited the queue length

@@ -55,6 +59,8 @@ public class TableRecords implements java.io.Serializable {

private List<Row> rows = new ArrayList<Row>();

private static final Interner<String> TABLE_NAME_POOL = Interners.newWeakInterner();
Copy link
Member

Choose a reason for hiding this comment

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

where used?

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

@slievrly slievrly merged commit 28b3413 into apache:develop Feb 28, 2023
Bughue added a commit to Bughue/seata that referenced this pull request Feb 28, 2023
@@ -120,20 +98,18 @@ private void init(DataSource dataSource, String resourceGroupId) {
}
initResourceId();
DefaultResourceManager.get().registerResource(this);
if (ENABLE_TABLE_META_CHECKER_ENABLE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

原定时刷新缓存的功能在这次更新后失效了?

Copy link
Contributor

Choose a reason for hiding this comment

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

tableMeta缓存刷新失效? #6660

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/rm-datasource rm-datasource module type: bug Category issues or prs related to bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AT模式下数据库表中新建了一个字段,就会报空指针。
7 participants