List:Maria Storage Engine« Previous MessageNext Message »
From:Michael Widenius Date:October 31 2008 9:40am
Subject:Re: review of bug37737
View as plain text  
Hi!

>>>>> "Oleksandr" == Oleksandr Byelkin <Oleksandr> writes:

Oleksandr> Hi!
Oleksandr> Michael Widenius пишет:
>> Hi!
>> 
>> Here is a review of the bug37737.diff file
>> 37737: Maria: ma_test_loghandler_multiflush-t fails
>> 
>> === modified file 'storage/maria/ma_loghandler.c'
>> --- storage/maria/ma_loghandler.c	2008-07-09 09:02:27 +0000
>> +++ storage/maria/ma_loghandler.c	2008-07-22 23:13:27 +0000

<cut>

>> A shorter version is:
>> 
>> DBUG_ASSERT((buffer->prev_sent_to_disk == buffer->prev_buffer_offset ||
buffer-> prev_sent_to_disk != LSN_IMPOSSIBLE);
>> 
>> (Not critical, just maybe intersting way to rewrite it)
>> 
>> I don't however see how this criteria is enforced.
>> In translog_buffer_next() you do:
>> 
new_buffer-> prev_buffer_offset=
>> log_descriptor.buffers[old_buffer_no].offset;
>> 
>> But you don't set prev_sent_to_disk in the same function. Is there
>> something that gurantees that prev_sent_to_disk isn't LSN_IMPOSSIBLE
>> when translog_buffer_next() is called ?

Oleksandr> Yes, you are right on the very first loop over buffers and very bad luck
Oleksandr>  the ASSERT can be broken, so I'll remove it.

ok

>> 
>> @@ -2553,10 +2584,34 @@ static my_bool translog_buffer_flush(str
file-> is_sync= 0;
>> 
>> if (LSN_OFFSET(buffer->last_lsn) != 0)    /* if buffer->last_lsn is set */
>> -    translog_set_sent_to_disk(buffer->last_lsn,
>> -                              buffer->next_buffer_offset);
>> +  {
>> +    if (translog_prev_buffer_flush_wait(buffer))
>> +      DBUG_RETURN(0); /* some the thread flushed the buffer already */
>> +    translog_set_sent_to_disk(buffer);
>> +  }
>> else
>> translog_set_only_in_buffers(buffer->next_buffer_offset);
>> 
>> Why don't we flush the previous buffer if last_lsn is not set ?
>> I can understand why we don't have to flush the current buffer if lsn
>> is not set (we don't have anything complete to flush).

Oleksandr> We do not flush it even if it is set. We just wait until it will be
Oleksandr> flushed (we sure that it should happened because thread which closed
Oleksandr> buffer will flush (send it to disk) or some flush log request can do it
Oleksandr> faster). But if there was no LSN generated then it does not matter
Oleksandr> (because it can't have impact on the log flush/sync (commit)).

ok

Regards,
Monty
Thread
review of bug37737Michael Widenius8 Oct
  • Re: review of bug37737Oleksandr \"Sanja\" Byelkin10 Oct
    • Re: review of bug37737Michael Widenius31 Oct