Skip to content
This repository was archived by the owner on Mar 1, 2026. It is now read-only.

Add different compression choice#1310

Open
sunshine-Chun wants to merge 1 commit into
facebook:fb-mysql-8.0.28from
sunshine-Chun:fb-mysql-8.0.28-cp-compression
Open

Add different compression choice#1310
sunshine-Chun wants to merge 1 commit into
facebook:fb-mysql-8.0.28from
sunshine-Chun:fb-mysql-8.0.28-cp-compression

Conversation

@sunshine-Chun

Copy link
Copy Markdown
Contributor

Summary: when using clone to copy data in innodb instance and turn compression on. The speed is about 10 times slower than with compression off. The default compression algorithm used is zlib. Try to use other compression algorithm to tune the performance. From testing, zstd is twice faster than zlib

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

@sunshine-Chun

Copy link
Copy Markdown
Contributor Author

Need to fix test.

Comment thread plugin/clone/src/clone_plugin.cc Outdated
@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch 3 times, most recently from f2fbd59 to 30300c7 Compare May 12, 2023 22:24
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Comment thread plugin/clone/include/clone_client.h Outdated
Comment thread plugin/clone/include/clone_client.h Outdated
@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from 30300c7 to e6bc11e Compare May 16, 2023 06:31
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Comment thread plugin/clone/include/clone.h Outdated
Comment thread plugin/clone/include/clone_client.h Outdated
Comment thread plugin/clone/src/clone_client.cc Outdated
Comment thread plugin/clone/src/clone_client.cc Outdated
Comment thread plugin/clone/src/clone_plugin.cc Outdated
@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from e6bc11e to 05a9ffa Compare May 17, 2023 05:43
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

Comment thread sql/server_component/clone_protocol_service.cc Outdated
Comment thread plugin/clone/include/clone_client.h Outdated
Comment thread include/mysql/plugin_clone.h Outdated
@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from 05a9ffa to 0c725aa Compare May 17, 2023 17:02
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

@laurynas-biveinis laurynas-biveinis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@sunshine-Chun

sunshine-Chun commented May 18, 2023

Copy link
Copy Markdown
Contributor Author

Looks like we have memory leakage from valgrind test. Need to debug a bit. Will update here once find anything.

  at 0xA5A6795: malloc (vg_replace_malloc.c:381)
   by 0x90917AD: ZSTD_customMalloc (allocations.h:30)
   by 0x90917AD: ZSTD_createDCtx_internal (zstd_decompress.c:296)
   by 0x90917AD: ZSTD_createDCtx (zstd_decompress.c:312)
by 0x7700BEA: zstd_uncompress(mysql_zstd_compress_context*, unsigned char*, unsigned long, unsigned long*) [clone .__uniq.287700427806220246369010965997275247218] (my_compress.cc:232)
   by 0x77009DE: my_uncompress(mysql_compress_context*, unsigned char*, unsigned long, unsigned long*) (my_compress.cc:699)
   by 0x64D4473: net_read_compressed_packet(NET*, unsigned long&) [clone .__uniq.40191296941203724010080517394248553221] (net_serv.cc:2259)
   by 0x64D4364: my_net_read(NET*) (net_serv.cc:2297)
   by 0x85CBC4D: mysql_clone_get_response(THD*, MYSQL*, bool, unsigned int, unsigned char**, unsigned long*, unsigned long*) (clone_protocol_service.cc:545)

@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from 0c725aa to 13690e4 Compare May 19, 2023 22:52
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@sunshine-Chun

Copy link
Copy Markdown
Contributor Author

Memory leakage fixed.

@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from 13690e4 to a394a4f Compare May 19, 2023 22:57
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@laurynas-biveinis laurynas-biveinis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with minor comment formatting issue fixed

*net_length = 0;
*length = my_net_read(net);

/** The ZSTD object has been updated to include the newly created decompressor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not a Doxygen comment:

Suggested change
/** The ZSTD object has been updated to include the newly created decompressor
/* The ZSTD object has been updated to include the newly created decompressor
*length = my_net_read(net);

/** The ZSTD object has been updated to include the newly created decompressor
* context. Include the change in the net extension so that the resource can

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This and the next line: the comment style with leading stars is not used in MySQL:

Suggested change
* context. Include the change in the net extension so that the resource can
context. Include the change in the net extension so that the resource can
@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from a394a4f to abc04b7 Compare May 30, 2023 16:18
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from abc04b7 to 6d5db1f Compare May 30, 2023 17:41
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@sunshine-Chun sunshine-Chun force-pushed the fb-mysql-8.0.28-cp-compression branch from 6d5db1f to b79ea9f Compare June 2, 2023 16:48
@facebook-github-bot

Copy link
Copy Markdown

@sunshine-Chun has updated the pull request. You must reimport the pull request before landing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

3 participants