Bug #70164 Misleading comment for the recv_parse_or_apply_log_rec_body() function
Submitted: 27 Aug 2013 13:57 Modified: 28 Aug 2013 11:19
Reporter: Valeriy Kravchuk Email Updates:
Status: Verified Impact on me:
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:5.1, 5.5, 5.6 OS:Any
Assigned to: CPU Architecture:Any
Tags: comment, innodb, rename

[27 Aug 2013 13:57] Valeriy Kravchuk
In recent code of MySQL 5.1, 5.5 and 5.6 we still see the following initial comment in the log0recv.cc (example is from 5.6):

   1018 /*******************************************************************//**
   1019 Tries to parse a single log record body and also applies it to a page if
   1020 specified. File ops are parsed, but not applied in this function.
   1021 @return log record end, NULL if not a complete record */
   1022 static
   1023 byte*
   1024 recv_parse_or_apply_log_rec_body(
   1025 /*=============================*/
   1026         byte            type,   /*!< in: type */
   1027         byte*           ptr,    /*!< in: pointer to a buffer */
   1028         byte*           end_ptr,/*!< in: pointer to the buffer end */
   1029         buf_block_t*    block,  /*!< in/out: buffer block or NULL; if
   1030                                 not NULL, then the log record is
   1031                                 applied to the page, and the log
   1032                                 record should be complete then */
   1033         mtr_t*          mtr,    /*!< in: mtr or NULL; should be non-NULL
   1034                                 if and only if block is non-NULL */
   1035         ulint           space_id)

Note this: "File ops are parsed, but not applied in this function."

At the same time, later in the code we see:

   1313         case MLOG_FILE_RENAME:
   1314                 ptr = fil_op_log_parse_or_replay(ptr, end_ptr, type,
   1315                                                  space_id, 0);
   1316                 break;

As you can see, the fil_op_log_parse_or_replay() function is called with space_id (that is non-zero probably) in this case, while its leading comment says that record is replayed with non-zero value:

        byte*   ptr,            /*!< in: buffer containing the log record body,
                                or an initial segment of it, if the record does
                                not fir completely between ptr and end_ptr */
        byte*   end_ptr,        /*!< in: buffer end */
        ulint   type,           /*!< in: the type of this log record */
        ulint   space_id,       /*!< in: the space id of the tablespace in
                                question, or 0 if the log record should
                                only be parsed but not replayed */
        ulint   log_flags)      /*!< in: redo log flags
                                (stored in the page number parameter) */

So, now original comment is misleading asw this function had got a new "side effect".

How to repeat:
Read the code. Check https://bugs.launchpad.net/percona-server/+bug/1217002 for some results of this new code being used in an old way (that is now wrong).

Suggested fix:
Please, fix the comment so that it explain all things this function may do.
[28 Aug 2013 11:19] Umesh Shastry
Hello Valeriy,

Thank you for your bug report.
Verified as described(5.6 source).