Hi Libing Song, Thanks for the explanations, all clear now. "slb.songlibing--- via developers" <developers@lists.mariadb.org> writes:
The patch enables the feature by default, with a threshold of 128MB. We need to consider if that is appropriate, or if the feature should be disabled by default.
That is according to Alibaba's cloud ssd's throughput which is 350MB/s. So the copy may take 1/3s if the data is still in the page cache, otherwise it will take 2/3s. I hope binlog commit should not take more than 1/2s. It could take less time on fast local ssd, but still rename is much faster than copy.
Ah, thanks for the explanation. It sounds like a good rule of thumb, set it to make commits not take more than approximately 0.5 seconds.
If there is any problem, users can disable it with a huge value. I
Ok, I'm fine with this for now. The 4kB will always be reserved at the start of the trx cache, if I understand correctly, but the commit by rotate/rename is configurable, as you say.
Why do you remove update_binlog_end_pos() here? Doesn't the binlog dump threads need this update?
reset_binlog_end_pos() already did the same thing.
Aha, I see, thanks for the explanation.
But why do you subtract LOG_EVENT_HEADER_LEN when calculating pad_to_size?
pad_size is data size of Gtid_event, so it doesn't include the size of header. Maybe it is
Aha, got it, the length argument to Log_event::write_header() is exclusive the LOG_EVENT_HEADER_LEN bytes, thanks for the explanation.
-CREATE OR REPLACE TABLE t1 (a DATETIME) ENGINE=MyISAM; +CREATE OR REPLACE TABLE t1 (a DATETIME) ENGINE=InnoDB; +# Use the transaction to keep binlog cache temporary file large enough +BEGIN; INSERT INTO t1 SELECT NOW() FROM seq_1_to_6000; + SET max_tmp_session_space_usage = 64*1024; --error 200 SELECT * FROM information_schema.ALL_PLUGINS LIMIT 2; +ROLLBACK; drop table t1; Can you explain why you need these changes to the test case? I was not able to understand that from the patch.
With reserved size, it exceeds CACHE_FILE_TRUNC_SIZE, thus binlog cache is truncated after commit. so keep the transaction to keep binlog cache large enough
Oh, I see. Agree, this is better, it seems somewhat accidental that the original test case worked. Nice find! :-) - Kristian.