maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Andrei!

Below is for a latest diff, 4927bf2534 f66893df77.

A couple of cosmetic comments, there's no need to do any further
reviews, thanks! I think it's ok to push as far as review is concerned.
(may be it's not ok from the testing poing of view, or may be it is -
I do not know)


> diff --git a/mysql-test/include/have_rocksdb.inc b/mysql-test/include/have_rocksdb.inc
> --- a/mysql-test/include/have_rocksdb.inc
> +++ b/mysql-test/include/have_rocksdb.inc
> @@ -1,4 +1,4 @@
> -if (`SELECT COUNT(*) = 0 FROM INFORMATION_SCHEMA.ENGINES WHERE engine = 'ROCKSDB' AND support IN ('YES', 'DEFAULT', 'ENABLED')`)
> +if (`SELECT COUNT(*) = 0 FROM INFORMATION_SCHEMA.PLUGINS WHERE PLUGIN_NAME = 'rocksdb'`)

why did you change that?

>  {
> -  --skip Test requires MyRocks engine
> -}
> +  --skip Requires rocksdb
> +}
> \ No newline at end of file
> diff --git a/sql/log.cc b/sql/log.cc
> --- a/sql/log.cc
> +++ b/sql/log.cc
...
> +  /* Change binlog file size to truncate_pos */
> +  if ((error=
> +       mysql_file_chsize(file, pos, 0, MYF(MY_WME))) ||
> +      (error= mysql_file_sync(file, MYF(MY_WME|MY_SYNC_FILESIZE))))

this if() is still overloaded, not debugger-friendly

> +  {
> +    sql_print_error("Failed to truncate the "
> +                    "binlog file:%s to size:%llu. Error:%d",
> +                    file_name, pos, error);
> +    goto end;
> +  }
> +  else
> +  {
> +    char buf[21];
> +    longlong10_to_str(ptr_gtid->seq_no, buf, 10);
> +    sql_print_information("Successfully truncated binlog file:%s "
> +                          "from previous file size %llu "
> +                          "to pos:%llu to remove transactions starting from "
> +                          "GTID %u-%u-%s",
> +                          file_name, old_size, pos,
> +                          ptr_gtid->domain_id, ptr_gtid->server_id, buf,
> +                          old_size);

you don't need old_size at the end, there's no format parameter for it.

> +  }
> +
> +end:
> +  if (file >= 0)
> +    mysql_file_close(file, MYF(MY_WME));
> +
> +  error= error || close_purge_index_file();
> +#endif
> +  return error > 0;
> +}
>  int TC_LOG_BINLOG::open(const char *opt_name)
>  {
>    int      error= 1;

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx