after-merge fixes and comments

mysql-test/include/maria_empty_logs.inc:
  At one moment in maria-recovery.test the first log has number 2,
  because log 1 was manually deleted.
mysql-test/r/maria-recovery.result:
  after-merge fix
mysql-test/t/maria-recovery.test:
  after-merge fix
storage/maria/ma_bitmap.c:
  after-merge fix. The todo is implemented now.
storage/maria/ma_blockrec.c:
  comment
storage/maria/ma_open.c:
  after-merge fix. Set write_fail also for index file or a write error
  would crash.
storage/maria/ma_pagecache.c:
  comment
storage/maria/ma_pagecache.h:
  I prefer to use NULL for 'no callback' instead of a dummy callback
  in the special case of get_log_address; indeed for non-transactional
  tables it uses an if(), while if using a dummy callback, it would
  use a function call plus an if() (the dummy callback would need to
  return a magic value to say "don't flush" and that value would be
  tested in if()).
storage/maria/unittest/ma_test_all-t:
  fix if running from outside storage/maria

mysql-test/include/maria_empty_logs.inc

@@ -17,6 +17,7 @@ if (!$mel_keep_control_file)
 {
   remove_file $MYSQLTEST_VARDIR/master-data/maria_log_control;
 }
+-- error 0,1
 remove_file $MYSQLTEST_VARDIR/master-data/maria_log.00000001;
 -- error 0,1
 remove_file $MYSQLTEST_VARDIR/master-data/maria_log.00000002;

mysql-test/r/maria-recovery.result

@@ -214,6 +214,9 @@ t1	CREATE TABLE `t1` (
   KEY `c` (`c`)
 ) ENGINE=MARIA AUTO_INCREMENT=16 DEFAULT CHARSET=latin1 PAGE_CHECKSUM=1
 drop table t1;
+* TEST of removing logs manually
+* shut down mysqld, removed logs, restarted it
+use mysqltest;
 * TEST of UNDO_ROW_DELETE preserving rowid
 create table t1(a int) engine=maria;
 insert into t1 values(1),(2);
@@ -222,17 +225,38 @@ flush table t1;
 lock tables t1 write;
 insert into t1 values(3);
 delete from t1 where a in (1,2,3);
-drop table t1;
+SET SESSION debug="+d,maria_flush_whole_log,maria_crash";
-* TEST of removing logs manually
+* crashing mysqld intentionally
-* shut down mysqld, removed logs, restarted it
+set global maria_checkpoint_interval=1;
+ERROR HY000: Lost connection to MySQL server during query
+* recovery happens
+check table t1 extended;
+Table	Op	Msg_type	Msg_text
+mysqltest.t1	check	status	OK
+* testing that checksum after recovery is as expected
+Checksum-check
+ok
 use mysqltest;
-create table t1 (a varchar(1000)) engine=maria;
+drop table t1;
-insert into t1 values ("00000000");
+* TEST of checkpoint
+set global debug="+d,info,query,enter,exit,loop,maria_checkpoint_indirect";
+set global maria_checkpoint_interval=10000;
+create table t1(a int, b varchar(10), index(a,b)) engine=maria;
+insert into t1 values(1,"a"),(2,"b"),(3,"c");
+delete from t1 where b="b";
+update t1 set b="d" where a=1;
 flush table t1;
 * copied t1 for comparison
 lock tables t1 write;
-insert into t1 values ("aaaaaaaaa");
+insert into t1 values(4,"e"),(5,"f"),(6,"g");
-SET SESSION debug="+d,maria_flush_whole_log,maria_crash";
+update t1 set b="h" where a=5;
+delete from t1 where b="g";
+show status like "Maria_pagecache_blocks_not_flushed";
+Variable_name	Value
+Maria_pagecache_blocks_not_flushed	3
+set global maria_checkpoint_interval=10000;
+update t1 set b="i" where a=5;
+SET SESSION debug="+d,maria_crash";
 * crashing mysqld intentionally
 set global maria_checkpoint_interval=1;
 ERROR HY000: Lost connection to MySQL server during query

mysql-test/t/maria-recovery.test

@@ -205,6 +205,35 @@ delete from t1 where a in (1,2,3);
 -- source include/maria_verify_recovery.inc
 drop table t1;
 
+# A basic checkpoint test
+--echo * TEST of checkpoint
+# Don't take a full checkpoints, we want to test checkpoint vs dirty pages
+set global debug="+d,info,query,enter,exit,loop,maria_checkpoint_indirect";
+# restart checkpoint thread for it to notice the above
+set global maria_checkpoint_interval=10000;
+create table t1(a int, b varchar(10), index(a,b)) engine=maria;
+insert into t1 values(1,"a"),(2,"b"),(3,"c");
+delete from t1 where b="b";
+update t1 set b="d" where a=1;
+-- source include/maria_make_snapshot_for_comparison.inc
+lock tables t1 write;
+insert into t1 values(4,"e"),(5,"f"),(6,"g");
+update t1 set b="h" where a=5;
+delete from t1 where b="g";
+show status like "Maria_pagecache_blocks_not_flushed";
+# force a checkpoint; there should be dirty pages and an open transaction
+set global maria_checkpoint_interval=10000;
+# do some more work
+update t1 set b="i" where a=5;
+let $mvr_restore_old_snapshot=0;
+let $mms_compare_physically=0;
+let $mvr_debug_option="+d,maria_crash";
+let $mvr_crash_statement= set global maria_checkpoint_interval=1;
+# Now we have a recovery, which should use the checkpoint record
+# and its dirty pages list.
+-- source include/maria_verify_recovery.inc
+drop table t1;
+
 # clean up everything
 let $mms_purpose=feeding_recovery;
 eval drop database mysqltest_for_$mms_purpose;

storage/maria/ma_bitmap.c

@@ -161,18 +161,6 @@ static inline my_bool write_changed_bitmap(MARIA_SHARE *share,
   }
   else
   {
-    /**
-      @todo RECOVERY BUG
-      Not flushable: its content is not reflected by the log, to honour WAL we
-      must keep the bitmap page pinned. Scenario of INSERT:
-      REDO - UNDO (written to log but not forced)
-      bitmap goes to page cache (because other INSERT needs to)
-      and then to disk (pagecache eviction)
-      crash: recovery will not find REDO-UNDO, table is corrupted.
-      Solutions:
-      give LSNs to bitmap pages or change pagecache to flush all log when
-      flushing a bitmap page or keep bitmap page pinned until checkpoint.
-    */
     MARIA_PINNED_PAGE page_link;
     int res= pagecache_write(share->pagecache,
                              &bitmap->file, bitmap->page, 0,
@@ -221,7 +209,6 @@ my_bool _ma_bitmap_init(MARIA_SHARE *share, File file)
 
   bitmap->block_size= share->block_size;
   bitmap->file.file= file;
-  bitmap->file.write_fail= &maria_page_write_failure; aaaaa
   _ma_bitmap_set_pagecache_callbacks(&bitmap->file, share);
 
   /* Size needs to be aligned on 6 */
@@ -2196,6 +2183,11 @@ void _ma_bitmap_flushable(MARIA_HA *info, int non_flushable_inc)
     info->non_flushable_state= 0;
     if (--bitmap->non_flushable == 0)
     {
+      /*
+        We unlock and unpin pages locked and pinned by other threads. It does
+        not seem to be an issue as all bitmap changes are serialized with
+        the bitmap's mutex.
+      */
       _ma_bitmap_unpin_all(share);
       if (unlikely(bitmap->flush_all_requested))
       {
@@ -2610,12 +2602,15 @@ void _ma_bitmap_set_pagecache_callbacks(PAGECACHE_FILE *file,
 {
   if (share->temporary)
     pagecache_file_init(*file, &maria_page_crc_check_none,
-                        &maria_page_filler_set_none, NULL, share);
+                        &maria_page_filler_set_none,
+                        &maria_page_write_failure,
+                        NULL, share);
   else
     pagecache_file_init(*file, &maria_page_crc_check_bitmap,
                         ((share->options & HA_OPTION_PAGE_CHECKSUM) ?
                          &maria_page_crc_set_normal :
                          &maria_page_filler_set_bitmap),
+                        &maria_page_write_failure,
                         share->now_transactional ?
                         &_ma_bitmap_get_log_address : NULL, share);
 }

storage/maria/ma_blockrec.c

@@ -446,7 +446,13 @@ my_bool _ma_once_end_block_record(MARIA_SHARE *share)
     share->bitmap.file.file= -1;
   }
   if (share->id != 0)
+  {
+    /*
+      We de-assign the id even though index has not been flushed, this is ok
+      as intern_lock serializes us with a Checkpoint looking at our share.
+    */
     translog_deassign_id_from_share(share);
+  }
   return res;
 }
 

storage/maria/ma_open.c

@@ -1540,15 +1540,17 @@ void set_data_pagecache_callbacks(PAGECACHE_FILE *file, MARIA_SHARE *share)
     them. On the other hand, index file can always have page CRCs, for all
     data formats.
   */
-  file->write_fail= &maria_page_write_failure;
   if (share->temporary)
     pagecache_file_init(*file, &maria_page_crc_check_none,
-                        &maria_page_filler_set_none, NULL, share);
+                        &maria_page_filler_set_none,
+                        &maria_page_write_failure,
+                        NULL, share);
   else
     pagecache_file_init(*file, &maria_page_crc_check_data,
                         ((share->options & HA_OPTION_PAGE_CHECKSUM) ?
                          &maria_page_crc_set_normal :
                          &maria_page_filler_set_normal),
+                        &maria_page_write_failure,
                         share->now_transactional ?
                         &maria_page_get_lsn : NULL, share);
 }
@@ -1556,15 +1558,17 @@ void set_data_pagecache_callbacks(PAGECACHE_FILE *file, MARIA_SHARE *share)
 
 void set_index_pagecache_callbacks(PAGECACHE_FILE *file, MARIA_SHARE *share)
 {
-  no write_fail set here?
   if (share->temporary)
     pagecache_file_init(*file, &maria_page_crc_check_none,
-                        &maria_page_filler_set_none, NULL, share);
+                        &maria_page_filler_set_none,
+                        &maria_page_write_failure,
+                        NULL, share);
   else
     pagecache_file_init(*file, &maria_page_crc_check_index,
                         ((share->options & HA_OPTION_PAGE_CHECKSUM) ?
                          &maria_page_crc_set_index :
                          &maria_page_filler_set_normal),
+                        &maria_page_write_failure,
                         share->now_transactional ?
                         &maria_page_get_lsn : NULL,
                         share);

storage/maria/ma_pagecache.c

@@ -3683,6 +3683,12 @@ static int flush_cached_blocks(PAGECACHE *pagecache,
        @todo IO If page is contiguous with next page to flush, group flushes
        in one single my_pwrite().
     */
+    /*
+      It is important to use block->hash_link->file below and not 'file', as
+      the first one is right and the second may have different content (and
+      this matters for callbacks, bitmap pages and data pages have different
+      ones).
+    */
     error= pagecache_fwrite(pagecache, &block->hash_link->file,
                             block->buffer,
                             block->hash_link->pageno,
@@ -3741,6 +3747,10 @@ static int flush_cached_blocks(PAGECACHE *pagecache,
    @param  filter_arg      an argument to pass to 'filter'. Information about
                            the block will be passed too.
 
+   @note
+     Flushes all blocks having the same OS file descriptor as 'file->file', so
+     can flush blocks having '*block->hash_link->file' != '*file'.
+
    @note
      This function doesn't do any mutex locks because it needs to be called
      both from flush_pagecache_blocks and flush_all_key_blocks (the later one

storage/maria/ma_pagecache.h

@@ -88,7 +88,7 @@ typedef struct st_pagecache_file
   my_bool (*write_callback)(uchar *page, pgcache_page_no_t offset,
                             uchar *data);
   void (*write_fail)(uchar *data);
-  /** Can be NULL */ or use dummy
+  /** Can be NULL */
   TRANSLOG_ADDRESS (*get_log_address_callback)
     (uchar *page, pgcache_page_no_t offset, uchar *data);
   uchar *callback_data;

storage/maria/unittest/ma_test_all-t

@@ -352,14 +352,14 @@ sub run_tests_on_clrs
            "rm test2.MA?"
           );
 
-  my @t3= ("ma_test2 -s -L -K -W -P -M -T -c -b32768 -t4 -A1",
+  my @t3= ("$maria_path/ma_test2 -s -L -K -W -P -M -T -c -b32768 -t4 -A1",
-           "maria_read_log -a -s",
+           "$maria_path/maria_read_log -a -s",
-           "maria_chk -es test2",
+           "$maria_path/maria_chk -es test2",
-           "maria_read_log -a -s",
+           "$maria_path/maria_read_log -a -s",
-           "maria_chk -es test2",
+           "$maria_path/maria_chk -es test2",
            "rm test2.MA?",
-           "maria_read_log -a -s",
+           "$maria_path/maria_read_log -a -s",
-           "maria_chk -es test2",
+           "$maria_path/maria_chk -es test2",
            "rm test2.MA?"
           );