Code review comment for lp:~hrvojem/percona-xtradb-cluster/pxc-12

Revision history for this message
Raghavendra D Prabhu (raghavendra-prabhu) wrote :

> "The downside of `mysqldump` and `rsync` is that the cluster becomes *READ-
> ONLY* while data is being copied from one node to another (SST applies
> :command:`FLUSH TABLES WITH READ LOCK` command). "

> The cluster becomes read-only, or the Donor node? I think only the Donor node
> (but I could be wrong, I haven't done much with mysqldump and rsync.

Thats right, by default only the donor node becomes read-only.
However, keep in mind this parameter:
gcs.sync_donor Should the rest of the cluster keep in sync with the donor? “Yes” means that if the donor is blocked by state transfer, the whole cluster is blocked with it.

from http://www.codership.com/wiki/doku.php?id=galera_parameters
(may be you can reference here to the variable index we have)

Now, with Xtrabackup too, it will be read-only too but for a
shorter while:

=====================================================================
        # make a prep copy before locking tables, if using rsync
        backup_files(1);

        # flush tables with read lock
        mysql_lockall();
    }

    if ($option_slave_info) {
        write_slave_info();
    }

    # backup non-InnoDB files and tables
    # (or finalize the backup by syncing changes if using rsync)
    backup_files(0);

    # resume ibbackup and wait till log copying is finished
    resume_ibbackup();

    # release read locks on all tables
    mysql_unlockall() if !$option_no_lock;

=====================================================================

So, under FTWRL, Xtrabackup does:

    1) Write binlog info, galera info and slave info

    2) A lighter rsync (which is a good optimization)

    3) Resume the ibbackup for copying the log files from a
    particular checkpoint

Along same lines, rsync SST does:

    a) FTWRL
    b) Disable writes (with google' patch)
    c) Full rsync
    d) Allow writes
    e) Unlock tables

Now, if you compare both. in Xtrabackup 2 + 3 should take less
time than c in rsync SST, hence being under FTWRL for a shorter
while. (It is better if Xtrabackup devs/AlexeyK also review the
above).

So, I think rather than saying read-only only for mysqldump and
rsync SST, we should compare the duration of FTWRL in both and
hence, the duration upto which the node is read-locked.

> "+ $ yum install Percona-XtraDB-Cluster-server Percona-XtraDB-Cluster-client
> xtrabackup"

> xtrabackup is a dependency of PXC server, AFAIK.

> "+ wsrep_cluster_address=gcomm://192.168.70.72,192.168.70.73"

> Actually, there's no reason why node1's ip can't be in the list, Galera is
> smart enough to remove it -- and this gives you a setting that you can make
> identical on all the nodes.

> Do we need innodb_locks_unsafe_for_binlog?

> I wouldn't necessarily recommend using root for SST, but a separate user.
> Include a link to the xtrabackup doc about grants necessary.

> Why are you not setting wsrep_cluster_name?

> "40 +.. note::
> 441 +
> 442 + Starting the cluster using the ``/etc/init.d/myslq start --wsrep-
> cluster-address="gcomm://"`` will only work on RedHat based distributions
> (like CentOS). This method won't work on Debian/Ubuntu due to difference in
> the init script.
> 443 +"

> Spelling of 'mysql' ^^
> How would you bootstrap if you can't use the init script?

> "+This method uses :program:`rsync` to copy files from donor to the joining
> node. In some cases this can be faster than using the |XtraBackup| but
> requires the global data lock which will block writes to the cluster. This
> method doesn't require username/password credentials to be set up in the
> variable :variable:`wsrep_sst_auth`."

> - Again, I think you mean 'node' here, not 'cluster'.

Agree with the rest.

> This is already described in the 3rd paragraph on that page:
> http://www.percona.com/doc/percona-xtradb-cluster/manual/bootstrap.html

>Then I suggest linking to it there --- "you can also bootstrap by modifying the config file as >described at this link..."

I think linking should be fine here.

review: Needs Fixing

« Back to merge proposal