Re: [Maria-developers] f3603cbfaeb: report progress
Hi, Nikita! On Nov 15, Nikita Malyavin wrote:
revision-id: f3603cbfaeb (mariadb-10.5.2-483-gf3603cbfaeb) parent(s): 3b6038bee88 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2021-01-29 03:47:52 +1000 message:
report progress
Perhaps, squash this with your commit "test progress reporting" ?
diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 29d0f487303..87dbe226c2e 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -11300,6 +11300,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi,
IO_CACHE *log_file= log->flip();
+ thd_progress_report(thd, 0, my_b_write_tell(log_file) + / rgi->tables_to_lock->m_conv_table->s->reclength);
1. why do you do this "/ rgi->...->s->reclength" ? 2. How does it work with your "cache flipping"? While data is copied between tables, concurrent connections write into an IO_CACHE. Then you flip, they what, write into the second IO_CACHE? What file length will you use here in thd_progress_report()?
+ do { const auto *descr_event= rgi->rli->relay_log.description_event_for_exec; @@ -11319,6 +11322,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi, free_root(&event_mem_root, MYF(MY_KEEP_PREALLOC)); if (ev != rgi->rli->relay_log.description_event_for_exec) delete ev; + thd_progress_report(thd, my_b_tell(log_file) + / rgi->tables_to_lock->m_conv_table->s->reclength, + thd->progress.max_counter); } while(!error);
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
On Mon, 15 Nov 2021 at 15:43, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Nikita!
On Nov 15, Nikita Malyavin wrote:
revision-id: f3603cbfaeb (mariadb-10.5.2-483-gf3603cbfaeb) parent(s): 3b6038bee88 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2021-01-29 03:47:52 +1000 message:
report progress
Perhaps, squash this with your commit "test progress reporting" ?
sure, after the review.
diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 29d0f487303..87dbe226c2e 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -11300,6 +11300,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi,
IO_CACHE *log_file= log->flip();
+ thd_progress_report(thd, 0, my_b_write_tell(log_file) + / rgi->tables_to_lock->m_conv_table->s->reclength);
1. why do you do this "/ rgi->...->s->reclength" ?
The idea was to write an approximate number of rows. I think it can come
in handy.
2. How does it work with your "cache flipping"? While data is copied between tables, concurrent connections write into an IO_CACHE. Then you flip, they what, write into the second IO_CACHE? What file length will you use here in thd_progress_report()?
Should start from zero again.. I dont't see the stage change there, maybe I wasn't sure should it be another stage or the same one.
Also, I see that result in another commit is wrong. Will fix soon,
SELECT progress from INFORMATION_SCHEMA.PROCESSLIST where id = 4; returns empty result set. Maybe id is not always 4.
+ do { const auto *descr_event= rgi->rli->relay_log.description_event_for_exec; @@ -11319,6 +11322,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi, free_root(&event_mem_root, MYF(MY_KEEP_PREALLOC)); if (ev != rgi->rli->relay_log.description_event_for_exec) delete ev; + thd_progress_report(thd, my_b_tell(log_file) + / rgi->tables_to_lock->m_conv_table->s->reclength, + thd->progress.max_counter); } while(!error);
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
-- Yours truly, Nikita Malyavin
Updated, see f1af51e46fec <https://github.com/MariaDB/server/commit/f1af51e46fec6b4b166b6afd6428286db2b43ff3> for changes, or bb-10.6-online-alter <https://github.com/MariaDB/server/commits/bb-10.6-online-alter> branch
diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 29d0f487303..87dbe226c2e 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -11300,6 +11300,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi,
IO_CACHE *log_file= log->flip();
+ thd_progress_report(thd, 0, my_b_write_tell(log_file) + / rgi->tables_to_lock->m_conv_table->s->reclength);
1. why do you do this "/ rgi->...->s->reclength" ?
The idea was to write an approximate number of rows. I think it can come in handy.
Well, I notice now that progress is in percents. I wonder if there's a way to get a raw value for a user.
Should start from zero again.. I dont't see the stage change there, maybe I wasn't sure should it be another stage or the same one.
Added in the new commit
Also, I see that result in another commit is wrong. Will fix soon,
The same commit, actually. I said "in another commit" because I was confused the test is not on email.
SELECT progress from INFORMATION_SCHEMA.PROCESSLIST where id = 4; returns empty result set. Maybe id is not always 4.
Now that's fixed, too.
-- Yours truly, Nikita Malyavin
Hi, Nikita! On Nov 24, Nikita Malyavin wrote:
diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 29d0f487303..87dbe226c2e 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -11300,6 +11300,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi,
IO_CACHE *log_file= log->flip();
+ thd_progress_report(thd, 0, my_b_write_tell(log_file) + / rgi->tables_to_lock->m_conv_table->s->reclength);
1. why do you do this "/ rgi->...->s->reclength" ?
The idea was to write an approximate number of rows. I think it can come in handy.
Well, I notice now that progress is in percents. I wonder if there's a way to get a raw value for a user.
I thought it's always in percents, that's why I wondered why you divided the value by a constant :)
Should start from zero again.. I dont't see the stage change there, maybe I wasn't sure should it be another stage or the same one.
Added in the new commit
Okay, good. You cannot have a progress going from 0% to 100% and then again fron 0% to 100% all in the same stage, that's not what users expect from a progress meter. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
participants (2)
-
Nikita Malyavin -
Sergei Golubchik