[Maria-developers] JIRA ping
Hi Kristian, I light of recent "I did see any of your comments until by accident just now", I now mail you directly that I updated both MDEV-162 and MDEV-7257. I want (review) comments. /Jonas
Jonas Oreland <jonaso@google.com> writes:
I light of recent "I did see any of your comments until by accident just now", I now mail you directly that I updated both MDEV-162 and MDEV-7257.
Thanks. I'll definitely look into it, I will try to do it as soon as possible. But it might be afew days, as things look to become busy now with optimistic parallel replication and opcoming 10.1.2 release. - Kristian.
Jonas Oreland <jonaso@google.com> writes:
I light of recent "I did see any of your comments until by accident just now", I now mail you directly that I updated both MDEV-162 and MDEV-7257.
Sorry for the late answer, I do not follow _all_ Jira activity, so I did not see your comments on those issues (I do read all of maria-developers@ though). I think the patch looks good. That binlog send code has long needed a cleanup, lots of duplicated code in there and other ugly stuff. A few comments:
diff --git a/mysql-test/suite/rpl/r/rpl_checksum.result b/mysql-test/suite/rpl/r/rpl_checksum.result index d88258f..b736dbc 100644 --- a/mysql-test/suite/rpl/r/rpl_checksum.result +++ b/mysql-test/suite/rpl/r/rpl_checksum.result
-Last_IO_Error = 'Got fatal error 1236 from master when reading data from binary log: 'Slave can not handle replication events with the checksum that master is configured to log; the first event 'master-bin.000009' at 367, the last event read from 'master-bin.000010' at 248, the last byte read from 'master-bin.000010' at 248.'' +Last_IO_Error = 'Got fatal error 1236 from master when reading data from binary log: 'Slave can not handle replication events with the checksum that master is configured to log; the first event 'master-bin.000009' at 367, the last event read from 'master-bin.000010' at 367, the last byte read from 'master-bin.000010' at 411.''
Why does the binlog position change here? (It's not necessarily a problem, and usually it's a bad idea anyway for test cases to depend on binlog positions. But it was hard to follow the details of the patch due to so much code moving around, so I want to understand why...)
diff --git a/sql/log.cc b/sql/log.cc index 2c20ea3..d10c1ad 100644 --- a/sql/log.cc +++ b/sql/log.cc
+ /** + * TODO(jonaso): Check with Kristian, should last_commit_pos really + * be updated for relay logs!!! + */ + if (!is_relay_log) + { + /* update binlog_end_pos so that it can be read by after sync hook */ + reset_binlog_end_pos(log_file_name, offset); + } + mysql_mutex_lock(&LOCK_commit_ordered); strmake_buf(last_commit_pos_file, log_file_name); - last_commit_pos_offset= my_b_tell(&log_file); + last_commit_pos_offset= offset; mysql_mutex_unlock(&LOCK_commit_ordered);
I agree, it is not necessary to update last_commit_pos for relay log. So it could be moved into if (!is_relay_log) { ... }.
@@ -4797,6 +4815,10 @@ void MYSQL_BIN_LOG::make_log_name(char* buf, const char* log_ident)
bool MYSQL_BIN_LOG::is_active(const char *log_file_name_arg) { + /** + * there should/must be a mysql_mutex_assert_owner(&LOCK_log) here... + * but code violates this all over! (scary monsters and super creeps!) + */ return !strcmp(log_file_name, log_file_name_arg); }
Hm. I guess this isn't a new problem? Though with the reduced locking of LOCK_log, it might have a higher risk of triggering? But if I understand the patch correctly, then it actually fixes this with respect to the binlog. Because now it uses the new binlog_end_pos, it seems there are no more calls to is_active() on the master. And it looks like on the slave side, it does hold LOCK_log while doing is_active(). So is this comment still appropriate? And if so, maybe it could be clarified instead to specify exactly where the code is that violates?
@@ -7387,6 +7434,10 @@ MYSQL_BIN_LOG::trx_group_commit_leader(group_commit_entry *leader)
DEBUG_SYNC(leader->thd, "commit_before_get_LOCK_commit_ordered"); mysql_mutex_lock(&LOCK_commit_ordered); + /** + * TODO(jonaso): Check with Kristian, + * if we rotate:d above, this offset is "wrong" + */ last_commit_pos_offset= commit_offset;
Agree, good catch, I think. If we rotate, we already update last_commit_pos_file and _offset. So the code needs to remember if it rotated, and skip the errorneous offset update in this case. Do you want to fix this, as part of the patch? Or should I take it?
@@ -811,6 +817,67 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG
+ mysql_mutex_assert_not_owner(&LOCK_binlog_end_pos); + lock_binlog_end_pos();
You shouldn't need the assert_not_owner() I think, safe_mutex will already complain if a thread tries to multiple lock a mutex.
+ The difference between this and last_commit_pos_{file,offset} is that + the commit position is updated later. If semi-sync wait point is set + to WAIT_AFTER_SYNC, the commit pos is update after semi-sync-ack has + been received and the end point is updated after the write as it's needed + for the dump threads to be able to semi-sync the event. + */ + my_off_t binlog_end_pos; + char binlog_end_pos_file[FN_REFLEN];
Right... last_commit_pos is about commits in the storage engine. While binlog_end_pos is about writes to the binlog. Agree.
+ strncpy(info->start_log_file_name, log_ident, + sizeof(info->start_log_file_name)); + info->start_log_file_name[sizeof(info->start_log_file_name)-1]= 0;
I think the better way to do this in MySQL/MariaDB sources is strmake(info->start_log_file_name, log_ident, sizeof(info->start_log_file_name)-1); ------ So I think the patch looks good. And getting rid of LOCK_log for every event sent from master to a slave, that can't be bad. So let's include it. So how do we do it? You mentioned wanting to avoid large intrusive diff in your tree relative to MariaDB upstream, which makes sense. But I suppose this is a large change to put in stable 10.0... Is it sufficient to include in 10.1/10.2, so that any new changes (which will happen only on top of this patch) will not conflict? (I think I also will want it in my 10.0-based replication tree, which will not be pushed to 10.0 main, but now includes the new parallel replication stuff...). - Kristian.
Hi again, i addressed your comments and I uploaded 2 new patches to JIRA. 1) a new "complete patch" 2) a patch that is changes from v1 to v2. comments inline below: looking forward to more feedback /Jonas On Mon, Dec 8, 2014 at 2:01 PM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Jonas Oreland <jonaso@google.com> writes:
I light of recent "I did see any of your comments until by accident just now", I now mail you directly that I updated both MDEV-162 and MDEV-7257.
Sorry for the late answer, I do not follow _all_ Jira activity, so I did not see your comments on those issues (I do read all of maria-developers@ though).
I think the patch looks good. That binlog send code has long needed a cleanup, lots of duplicated code in there and other ugly stuff.
A few comments:
diff --git a/mysql-test/suite/rpl/r/rpl_checksum.result b/mysql-test/suite/rpl/r/rpl_checksum.result index d88258f..b736dbc 100644 --- a/mysql-test/suite/rpl/r/rpl_checksum.result +++ b/mysql-test/suite/rpl/r/rpl_checksum.result
-Last_IO_Error = 'Got fatal error 1236 from master when reading data from binary log: 'Slave can not handle replication events with the checksum that master is configured to log; the first event 'master-bin.000009' at 367, the last event read from 'master-bin.000010' at 248, the last byte read from 'master-bin.000010' at 248.'' +Last_IO_Error = 'Got fatal error 1236 from master when reading data from binary log: 'Slave can not handle replication events with the checksum that master is configured to log; the first event 'master-bin.000009' at 367, the last event read from 'master-bin.000010' at 367, the last byte read from 'master-bin.000010' at 411.''
Why does the binlog position change here? (It's not necessarily a problem, and usually it's a bad idea anyway for test cases to depend on binlog positions. But it was hard to follow the details of the patch due to so much code moving around, so I want to understand why...)
thanks! the problem was that I didn't update info->last_pos and linfo->pos correctly in send_format_descriptor_event so the offsets were wrong (in the old file)...how ever the result file still changes, since the old code was also incorrect :-) The new error is: Last_IO_Error = 'Got fatal error 1236 from master when reading data from binary log: 'Slave can not handle replication events with the checksum that master is configured to log; the first event 'master-bin.000009' at 367, the last event read from 'master-bin.000010' at 4, the last byte read from 'master-bin.000010' at 248.'' which is entirely correct, the last read event started at offset 4 and ended at offset 248...and the slave requested to start at master-bin.000009 at 367 --- this also made me find a bug that when seeking i forgot to update linfo->pos
diff --git a/sql/log.cc b/sql/log.cc index 2c20ea3..d10c1ad 100644 --- a/sql/log.cc +++ b/sql/log.cc
+ /** + * TODO(jonaso): Check with Kristian, should last_commit_pos really + * be updated for relay logs!!! + */ + if (!is_relay_log) + { + /* update binlog_end_pos so that it can be read by after sync hook */ + reset_binlog_end_pos(log_file_name, offset); + } + mysql_mutex_lock(&LOCK_commit_ordered); strmake_buf(last_commit_pos_file, log_file_name); - last_commit_pos_offset= my_b_tell(&log_file); + last_commit_pos_offset= offset; mysql_mutex_unlock(&LOCK_commit_ordered);
I agree, it is not necessary to update last_commit_pos for relay log. So it could be moved into if (!is_relay_log) { ... }.
done.
@@ -4797,6 +4815,10 @@ void MYSQL_BIN_LOG::make_log_name(char* buf, const char* log_ident)
bool MYSQL_BIN_LOG::is_active(const char *log_file_name_arg) { + /** + * there should/must be a mysql_mutex_assert_owner(&LOCK_log) here... + * but code violates this all over! (scary monsters and super creeps!) + */ return !strcmp(log_file_name, log_file_name_arg); }
Hm. I guess this isn't a new problem? Though with the reduced locking of LOCK_log, it might have a higher risk of triggering?
But if I understand the patch correctly, then it actually fixes this with respect to the binlog. Because now it uses the new binlog_end_pos, it seems there are no more calls to is_active() on the master.
And it looks like on the slave side, it does hold LOCK_log while doing is_active().
So is this comment still appropriate? And if so, maybe it could be clarified instead to specify exactly where the code is that violates
i'll recheck. ...time passes... ...checking... ...time passes... ok, I readded the assert, and the code crashed on the slave side. I updated comment with stack trace, but left it at that.
@@ -7387,6 +7434,10 @@ MYSQL_BIN_LOG::trx_group_commit_leader(group_commit_entry *leader)
DEBUG_SYNC(leader->thd, "commit_before_get_LOCK_commit_ordered"); mysql_mutex_lock(&LOCK_commit_ordered); + /** + * TODO(jonaso): Check with Kristian, + * if we rotate:d above, this offset is "wrong" + */ last_commit_pos_offset= commit_offset;
Agree, good catch, I think. If we rotate, we already update last_commit_pos_file and _offset. So the code needs to remember if it rotated, and skip the errorneous offset update in this case.
yes,
Do you want to fix this, as part of the patch? Or should I take it?
you take
@@ -811,6 +817,67 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG
+ mysql_mutex_assert_not_owner(&LOCK_binlog_end_pos); + lock_binlog_end_pos();
You shouldn't need the assert_not_owner() I think, safe_mutex will already complain if a thread tries to multiple lock a mutex.
i see it as easy to read documentation...
+ The difference between this and last_commit_pos_{file,offset} is that + the commit position is updated later. If semi-sync wait point is set + to WAIT_AFTER_SYNC, the commit pos is update after semi-sync-ack has + been received and the end point is updated after the write as it's needed + for the dump threads to be able to semi-sync the event. + */ + my_off_t binlog_end_pos; + char binlog_end_pos_file[FN_REFLEN];
Right... last_commit_pos is about commits in the storage engine. While binlog_end_pos is about writes to the binlog. Agree.
+ strncpy(info->start_log_file_name, log_ident, + sizeof(info->start_log_file_name)); + info->start_log_file_name[sizeof(info->start_log_file_name)-1]= 0;
I think the better way to do this in MySQL/MariaDB sources is
strmake(info->start_log_file_name, log_ident, sizeof(info->start_log_file_name)-1);
done
------
So I think the patch looks good. And getting rid of LOCK_log for every event sent from master to a slave, that can't be bad. So let's include it.
So how do we do it? You mentioned wanting to avoid large intrusive diff in your tree relative to MariaDB upstream, which makes sense. But I suppose this is a large change to put in stable 10.0... Is it sufficient to include in 10.1/10.2, so that any new changes (which will happen only on top of this patch) will not conflict?
(I think I also will want it in my 10.0-based replication tree, which will not be pushed to 10.0 main, but now includes the new parallel replication stuff...).
- Kristian.
i think i'm fine either way, put it wherever you like. (at least currently I'm mainly interested in the review feedback) /Jonas
Jonas Oreland <jonaso@google.com> writes: Thanks for the MDEV-162 patch! Here is my review. I think the patch looks fine, and we should take it. Just a few questions below, and one minor comment:
=== modified file 'mysql-test/suite/rpl/t/rpl_semi_sync.test' --- mysql-test/suite/rpl/t/rpl_semi_sync.test 2014-08-07 16:06:56 +0000 +++ mysql-test/suite/rpl/t/rpl_semi_sync.test 2014-10-20 10:52:49 +0000 @@ -59,7 +59,6 @@ echo [ status of semi-sync on master should be ON even without any semi-sync slaves ]; show status like 'Rpl_semi_sync_master_clients'; show status like 'Rpl_semi_sync_master_status'; ---replace_result 305 304 show status like 'Rpl_semi_sync_master_yes_tx';
I'm curious why you decided to remove this --replace_result? I see in bzr history that Monty added this --replace_result without any explanation why... :-/
=== added file 'mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.opt' --- mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.opt 1970-01-01 00:00:00 +0000 +++ mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.opt 2014-10-20 09:25:13 +0000 @@ -0,0 +1,1 @@ +--log_bin
Any reason not to use --source include/have_log_bin.inc instead?
=== added file 'mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.test' --- mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.test 1970-01-01 00:00:00 +0000 +++ mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.test 2014-10-20 09:25:13 +0000
+--echo # Kill the waiting thread; it should die immediately. +KILL @other_connection_id; + +--echo # Collect the error from the INSERT thread; it should be disconnected. +connection other; +--error 2013
Please use here: --error 2013,ER_CONNECTION_KILLED to avoid a rare test failure (apparently the ER_CONNECTION_KILLED can occasionally reach the client before the socket is closed, I've seen it in our Buildbot).
+--echo # Collect the error from the INSERT thread; it should be disconnected. +connection other; +--error 2013
Also here use --error 2013,ER_CONNECTION_KILLED
=== modified file 'sql/replication.h' --- sql/replication.h 2014-08-07 16:06:56 +0000 +++ sql/replication.h 2014-10-20 09:25:13 +0000
@@ -114,7 +117,13 @@ */ enum Binlog_storage_flags { /** Binary log was sync:ed */ - BINLOG_STORAGE_IS_SYNCED = 1 + BINLOG_STORAGE_IS_SYNCED = 1, + + /** First(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_LEADER = 2, + + /** Last(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_TRAILER = 4 };
What is the reason for introducing these flags? As far as I can see from the patch, they are set, but never read? Thanks, - Kristian.
Hi there, I think it might be easier you just took and applied it to your favorite branch (hence i'll won't send a new version of the patch). however there is one think that I discovered that needs to be fixed with the interaction with the Dump Thread Enhancement...I'll add that patch to this or the other JIRA entry. other comments inline below. thanks for review. /Jonas On Fri, Dec 12, 2014 at 12:08 PM, Kristian Nielsen <knielsen@knielsen-hq.org
wrote:
Jonas Oreland <jonaso@google.com> writes:
Thanks for the MDEV-162 patch! Here is my review.
I think the patch looks fine, and we should take it. Just a few questions below, and one minor comment:
=== modified file 'mysql-test/suite/rpl/t/rpl_semi_sync.test' --- mysql-test/suite/rpl/t/rpl_semi_sync.test 2014-08-07 16:06:56 +0000 +++ mysql-test/suite/rpl/t/rpl_semi_sync.test 2014-10-20 10:52:49 +0000 @@ -59,7 +59,6 @@ echo [ status of semi-sync on master should be ON even without any semi-sync slaves ]; show status like 'Rpl_semi_sync_master_clients'; show status like 'Rpl_semi_sync_master_status'; ---replace_result 305 304 show status like 'Rpl_semi_sync_master_yes_tx';
I'm curious why you decided to remove this --replace_result? I see in bzr history that Monty added this --replace_result without any explanation why... :-/
i removed it cause the result didn't contain 304 or 305 my guess is that it hasn't for several years, and hence it was pure obfuscation. i also don't know/understand why it was added in the first place, maybe because committer was too lazy to investigate?
=== added file 'mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.opt' --- mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.opt 1970-01-01 00:00:00 +0000 +++ mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.opt 2014-10-20 09:25:13 +0000 @@ -0,0 +1,1 @@ +--log_bin
Any reason not to use --source include/have_log_bin.inc instead?
hmm...damn it, don't remember... i guess it would work equally well (or even better from mtr.pl point of view)
=== added file 'mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.test'
--- mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.test 1970-01-01 00:00:00 +0000 +++ mysql-test/suite/rpl/t/rpl_semi_sync_wait_point.test 2014-10-20 09:25:13 +0000
+--echo # Kill the waiting thread; it should die immediately. +KILL @other_connection_id; + +--echo # Collect the error from the INSERT thread; it should be disconnected. +connection other; +--error 2013
Please use here:
--error 2013,ER_CONNECTION_KILLED
ack
to avoid a rare test failure (apparently the ER_CONNECTION_KILLED can occasionally reach the client before the socket is closed, I've seen it in our Buildbot).
+--echo # Collect the error from the INSERT thread; it should be disconnected. +connection other; +--error 2013
Aso here use --error 2013,ER_CONNECTION_KILLED
ack
=== modified file 'sql/replication.h' --- sql/replication.h 2014-08-07 16:06:56 +0000 +++ sql/replication.h 2014-10-20 09:25:13 +0000
@@ -114,7 +117,13 @@ */ enum Binlog_storage_flags { /** Binary log was sync:ed */ - BINLOG_STORAGE_IS_SYNCED = 1 + BINLOG_STORAGE_IS_SYNCED = 1, + + /** First(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_LEADER = 2, + + /** Last(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_TRAILER = 4 };
What is the reason for introducing these flags? As far as I can see from the patch, they are set, but never read?
this is for a (yet) unpublished optimization, that is to not sem-sync all individual transactions in a group-commit but only the last one (trailer) (and i added leader for completeness). performance impact is significant, i'll publish the patch+result later. i haven't ported http://my-replication-life.blogspot.se/2014/03/faster-semisync-replication.h... (since I found the implementation hackish by violating the vio-abstraction). but hope that optimization above will give about the same performance wise... /Jonas
Thanks,
- Kristian.
Jonas Oreland <jonaso@google.com> writes:
I think it might be easier you just took and applied it to your favorite branch (hence i'll won't send a new version of the patch).
Ok.
however there is one think that I discovered that needs to be fixed with the interaction with the Dump Thread Enhancement...I'll add that patch to this or the other JIRA entry.
Ok.
---replace_result 305 304 show status like 'Rpl_semi_sync_master_yes_tx';
I'm curious why you decided to remove this --replace_result? I see in bzr history that Monty added this --replace_result without any explanation why... :-/
i removed it cause the result didn't contain 304 or 305 my guess is that it hasn't for several years, and hence it was pure obfuscation.
Hehe, nice... thanks for the explanation.
Any reason not to use --source include/have_log_bin.inc instead?
hmm...damn it, don't remember... i guess it would work equally well (or even better from mtr.pl point of view)
Right, I was just curious, not that important.
+ /** First(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_LEADER = 2, + + /** Last(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_TRAILER = 4 };
What is the reason for introducing these flags? As far as I can see from the patch, they are set, but never read?
this is for a (yet) unpublished optimization, that is to not sem-sync all individual transactions in a group-commit but only the last one (trailer) (and i added leader for completeness).
Ok, that sounds cool! Thanks, - Kristian.
Hi again, uploaded dump-thread-enhancements.changes-v2-v3.patch which releases LOCK_binlog_end_pos while sending hb. and...to repeat, the dump-thread-enhancements is a prerequisite for this. /Jonas On Fri, Dec 12, 2014 at 1:56 PM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Jonas Oreland <jonaso@google.com> writes:
I think it might be easier you just took and applied it to your favorite branch (hence i'll won't send a new version of the patch).
Ok.
however there is one think that I discovered that needs to be fixed with the interaction with the Dump Thread Enhancement...I'll add that patch to this or the other JIRA entry.
Ok.
---replace_result 305 304 show status like 'Rpl_semi_sync_master_yes_tx';
I'm curious why you decided to remove this --replace_result? I see in bzr history that Monty added this --replace_result without any explanation why... :-/
i removed it cause the result didn't contain 304 or 305 my guess is that it hasn't for several years, and hence it was pure obfuscation.
Hehe, nice... thanks for the explanation.
Any reason not to use --source include/have_log_bin.inc instead?
hmm...damn it, don't remember... i guess it would work equally well (or even better from mtr.pl point of view)
Right, I was just curious, not that important.
+ /** First(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_LEADER = 2, + + /** Last(or alone) in a group commit */ + BINLOG_GROUP_COMMIT_TRAILER = 4 };
What is the reason for introducing these flags? As far as I can see from the patch, they are set, but never read?
this is for a (yet) unpublished optimization, that is to not sem-sync all individual transactions in a group-commit but only the last one (trailer) (and i added leader for completeness).
Ok, that sounds cool!
Thanks,
- Kristian.
Jonas Oreland <jonaso@google.com> writes:
uploaded dump-thread-enhancements.changes-v2-v3.patch which releases LOCK_binlog_end_pos while sending hb.
and...to repeat, the dump-thread-enhancements is a prerequisite for this.
Thanks! I've pushed the dump-thread enhancements v3 and the MDEV-162 patches to 10.1. - Kristian.
Jonas Oreland <jonaso@google.com> writes:
i addressed your comments and I uploaded 2 new patches to JIRA. 1) a new "complete patch" 2) a patch that is changes from v1 to v2.
the problem was that I didn't update info->last_pos and linfo->pos correctly in send_format_descriptor_event so the offsets were wrong (in the old file)...how ever the result file still changes, since the old code was also incorrect :-)
this also made me find a bug that when seeking i forgot to update linfo->pos
Ok, cool.
ok, I readded the assert, and the code crashed on the slave side. I updated comment with stack trace, but left it at that.
+ * there should/must be mysql_mutex_assert_owner(&LOCK_log) here... + * but code violates this! (scary monsters and super creeps!) + * + * example stacktrace: + * #8 MYSQL_BIN_LOG::is_active + * #9 MYSQL_BIN_LOG::can_purge_log + * #10 MYSQL_BIN_LOG::purge_logs + * #11 MYSQL_BIN_LOG::purge_first_log + * #12 next_event + * #13 exec_relay_log_event
Ok, I see. Right, seems there is a potential issue between the slave IO thread moving to a new relay log, and the SQL thread checking if it can purge an old log. Thanks for checking. At least the master side seems fixed now, with your patch.
you take
Ok, I filed it as MDEV-7310 so it does not get lost.
i think i'm fine either way, put it wherever you like. (at least currently I'm mainly interested in the review feedback)
Ok. I checked the "changes from v1 to v2 diff", and it looks fine to me now. I can apply it along with the other related patches. I can also give it a spin through our Buildbot first to check for any issues. - Kristian.
participants (2)
-
Jonas Oreland -
Kristian Nielsen