Skip to content

add Tidb provider module.#5844

Merged
kezhenxu94 merged 21 commits into
masterfrom
tidb_provider
Nov 22, 2020
Merged

add Tidb provider module.#5844
kezhenxu94 merged 21 commits into
masterfrom
tidb_provider

Conversation

@JaredTan95
Copy link
Copy Markdown
Member

@JaredTan95 JaredTan95 commented Nov 14, 2020

Fix #5754, #5749

  • Add a e2e test to verify it works.

@JaredTan95 JaredTan95 requested review from kezhenxu94 and wu-sheng and removed request for wu-sheng November 14, 2020 12:43
@JaredTan95 JaredTan95 added backend OAP backend related. bug Something isn't working and you are sure it's a bug! core feature Core and important feature. Sometimes, break backwards compatibility. labels Nov 14, 2020
@JaredTan95 JaredTan95 added this to the 8.3.0 milestone Nov 14, 2020
@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 14, 2020

Codecov Report

Merging #5844 (590bea7) into master (e8f30f9) will increase coverage by 0.02%.
The diff coverage is 85.52%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #5844      +/-   ##
============================================
+ Coverage     51.15%   51.18%   +0.02%     
- Complexity     3476     3486      +10     
============================================
  Files          1656     1659       +3     
  Lines         35577    35650      +73     
  Branches       3905     3910       +5     
============================================
+ Hits          18200    18246      +46     
- Misses        16469    16495      +26     
- Partials        908      909       +1     
Impacted Files Coverage Δ Complexity Δ
...storage/plugin/jdbc/h2/dao/H2HistoryDeleteDAO.java 14.28% <0.00%> (ø) 1.00 <0.00> (ø)
...storage/plugin/jdbc/tidb/TiDBHistoryDeleteDAO.java 81.81% <81.81%> (ø) 6.00 <6.00> (?)
.../storage/plugin/jdbc/tidb/TiDBStorageProvider.java 88.00% <88.00%> (ø) 9.00 <9.00> (?)
...brary/client/jdbc/hikaricp/JDBCHikariCPClient.java 63.38% <100.00%> (+12.67%) 13.00 <0.00> (+1.00)
.../storage/plugin/jdbc/mysql/MySQLStorageConfig.java 100.00% <100.00%> (ø) 1.00 <1.00> (ø)
...er/storage/plugin/jdbc/tidb/TiDBStorageConfig.java 100.00% <100.00%> (ø) 1.00 <1.00> (?)
...s/manual/database/DatabaseStatementDispatcher.java 10.00% <0.00%> (-90.00%) 1.00% <0.00%> (-1.00%)
...skywalking/oap/server/core/analysis/topn/TopN.java 0.00% <0.00%> (-33.34%) 0.00% <0.00%> (-1.00%)
...r/cluster/plugin/standalone/StandaloneManager.java 77.77% <0.00%> (-22.23%) 3.00% <0.00%> (-1.00%)
...ng/oap/server/core/analysis/worker/TopNWorker.java 42.42% <0.00%> (-12.13%) 4.00% <0.00%> (-1.00%)
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8f30f9...590bea7. Read the comment docs.

Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

  1. Seems you missed the license.
  2. I think you accidentally updated the UI submodule?
  3. Please add e2e for TiDB storage provider.

@xbkaishui
Copy link
Copy Markdown
Contributor

Due to tidb not do well on batch delete, is it better we use partition table?
tiflash is suitable for olap related task. do we need integrate it in e2e test case?

@wu-sheng
Copy link
Copy Markdown
Member

@JaredTan95 Please resolveconflict.

# Conflicts:
#	CHANGES.md
#	oap-server/server-storage-plugin/storage-jdbc-hikaricp-plugin/src/main/java/org/apache/skywalking/oap/server/storage/plugin/jdbc/h2/dao/H2HistoryDeleteDAO.java
@JaredTan95
Copy link
Copy Markdown
Member Author

There is some problem creating a new database when there starting with tidb. :-P

@kezhenxu94
Copy link
Copy Markdown
Member

kezhenxu94 commented Nov 18, 2020

There is some problem creating a new database when there starting with tidb. :-P

Need any help? Feel free to ping me if anything I can help

@JaredTan95
Copy link
Copy Markdown
Member Author

JaredTan95 commented Nov 18, 2020

There is some problem creating a new database when there starting with tidb. :-P

Need any help? Feel free to ping me if anything I can help

Yes,
Thank you first~
There is some problem in e2e test, due to we have not found a way to create sw db during tidb container creating.

@wu-sheng
Copy link
Copy Markdown
Member

@kezhenxu94 Anyway to inject an initial container?

kezhenxu94
kezhenxu94 previously approved these changes Nov 22, 2020
Copy link
Copy Markdown
Member

@kezhenxu94 kezhenxu94 left a comment

Choose a reason for hiding this comment

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

Tests passed. LGTM except for one nit inline. You may also want to add a case into the ttl case matrix

Comment thread oap-server/server-storage-plugin/storage-tidb-plugin/pom.xml Outdated
@kezhenxu94 kezhenxu94 requested a review from wu-sheng November 22, 2020 03:05
Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Could we add a TTL e2e for tidb too?

@JaredTan95
Copy link
Copy Markdown
Member Author

Could we add a TTL e2e for tidb too?

added.

Copy link
Copy Markdown
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

Once e2e passed, this is good for me.

@kezhenxu94 kezhenxu94 merged commit 8d164d3 into master Nov 22, 2020
@kezhenxu94 kezhenxu94 deleted the tidb_provider branch November 22, 2020 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend OAP backend related. bug Something isn't working and you are sure it's a bug! core feature Core and important feature. Sometimes, break backwards compatibility.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transaction is too large when use TiDB as storage

4 participants