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

Merge mydumper head + morgo changes #4

Merged
merged 51 commits into from
Oct 9, 2018
Merged

Conversation

morgo
Copy link

@morgo morgo commented Oct 7, 2018

This merges the changes from maxbube/mydumper and morgo/mydumper.

equinox0815 and others added 30 commits January 16, 2017 13:48
Signed-off-by: Christian Pointner <equinox@spreadspace.org>
This change leverage the MarkDown format in the file
MariaDB 10.2 splitted server/client variables and now
MYSQL_SERVER_VERSION is not available. We now take the
version string from the client header.

Patch provided by by Brian Evans (grknight@gentoo.org):
https://bugs.gentoo.org/635176

Upstream issue:
https://jira.mariadb.org/browse/MDEV-13773
… line (mydumper#15)

* adding the ability to use --ask-password or -a in order to be prompted to use a password rather than passing it through the command line

* Adjusting getPassword to use getpass() so the password isn't displayed to the user

* forgot to include the option for password prompting in myloader.c as well

* Forgot to include getPassword.c in the cmakelist in order to get it to work in myloader

* removing new lines

Removing new lines as requested
The patch tries to not use the server version string for the version
output.
MySQL has added LIBMYSQL_VERSION to mysql_version.h
MariaDB has added MARIADB_VERSION_NUMBER_STR via an include and no
longer declares MYSQL_SERVER_VERSION for the client.
…mydumper#75)

just by filename which mean the **CURRENT WORK DIR**, but it should
be the file in data directory passed by -d
Commit 37aa066 broke compilation under
at least Debian 9 and CentOS 6, as MYSQL_TYPE_JSON is not defined in the
MySQL versions they ship.
* New option to specify tables to skip via a file.

The --regex option works well but cannot scale to hundreds of thousands
of distinct table names when a large number of tables must not be backed
up.

This new --omit-from-file option allows specifying a file containing a
list of table names (in database.table format, one per line) to skip
during backup.

* Document the new --omit-from-file option.
* add support for mysql 5.7 generated fields

* minor memory leak fixes
* Fix memleak in dump_table_data

We've noticed a high memory usage when there were alot of tables/databases.
The memory usage increased while dumping the tables.
Valgrind gave us the answer:
==24877== 24,192,604 (1,757,400 direct, 22,435,204 indirect) bytes in 73,225 blocks are definitely lost in loss record 175 of 185
==24877==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==24877==    by 0x60267C9: g_malloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==24877==    by 0x603D62F: g_slice_alloc (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==24877==    by 0x60416C2: g_string_sized_new (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==24877==    by 0x41FCA0: dump_table_data (mydumper.c:2723)
==24877==    by 0x4207C7: dump_table_data_file (mydumper.c:2521)
==24877==    by 0x420D13: process_queue (mydumper.c:522)
==24877==    by 0x6047844: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4200.1)
==24877==    by 0x4E3D063: start_thread (pthread_create.c:309)

* Fix more memleaks
* TLS support

* DRY the connection code

* Add and use SSL verification options
* added option to disable ssl build

* remove ssl arguments when -DWITH_SSL=OFF

* vars from connection.c

* support for old MYSQL_SERVER_VERSION
@morgo
Copy link
Author

morgo commented Oct 7, 2018

I've manually tested this, and discovered an issue in TiDB pingcap/tidb#7833 - but it should otherwise work.

README.md Outdated
== How to build it? ==
## How to install mydumper/myloader?

First get the correct url from the [releases section](https://github.com/maxbube/mydumper/releases) then:
Copy link
Collaborator

Choose a reason for hiding this comment

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

could we change these link?

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the README just now to make it more clear what this fork does. The manual links will not work until pingcap/docs#644 is merged.

mydumper.c Outdated
}

// Need to set the @@tidb_snapshot for the master thread
gchar *query= g_strdup_printf("SET SESSION tidb_snapshot = '%s'", tidb_snapshot);
Copy link
Collaborator

Choose a reason for hiding this comment

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

add indentation

@IANTHEREAL
Copy link
Collaborator

Rest LGTM! exciting work!

@IANTHEREAL
Copy link
Collaborator

@kennytm PTAL

@kennytm
Copy link

kennytm commented Oct 8, 2018

LGTM.

(BTW I noticed an error while checking a1ddcba...e95223e:

mydumper/myloader.c

Lines 208 to 212 in 85fa642

count = scandir(directory, &namelist, 0, alphasort);
if (count < 0) {
g_critical("cannot open directory %s, error: %s\n", directory, strerror(count));
assert(0);
}

The strerror(count) should be strerror(errno))

@morgo
Copy link
Author

morgo commented Oct 8, 2018

@kennytm good catch. I've fixed it, we might as well.

@IANTHEREAL
Copy link
Collaborator

any more changes? if not, you can merge it @morgo

@kennytm kennytm removed their assignment Oct 9, 2018
@morgo
Copy link
Author

morgo commented Oct 9, 2018

@GregoryIan I do not have permissions. Please merge away :-)

@kennytm kennytm merged commit 654f3ba into pingcap:master Oct 9, 2018
@kennytm
Copy link

kennytm commented Oct 9, 2018

I've merged it for you 🙂 @GregoryIan @morgo

@morgo morgo deleted the tidb-head branch October 9, 2018 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet