Code review comment for lp:~percona-core/percona-server/t12-crash-safe-slave

> Hi George!
>
> I've looked at the patch and I have couple of questions.
>
> * MYSQL_SERVER is defined because we use some MySQL headers in
> ha_innodb.cc. Why do we use ifdef MYSQL_SERVER and #ifndef
> MYSQL_SERVER in ha_innodb.cc? We defined it unconditionally for
> this file, so ha_innodb.cc seems cannot be built MYSQL_CLIENT defined.

This was lifted directly from PS 5.5. I had wondered the same things but was unsure of why it was placed there originally but did not want to spend too much time investigating that. Maybe I can get Laurynas to comment on that.

> * line 845 of the diff seems just adds an empty line

Good catch, that was left behind from when I had debugging code in there. I thought I had everything scrubbed. I guess not :-\

> * can you clarify what is the Crash Safe Slave feature and why it
> cannot be fully replaced with our Crash Resistant Replication?
> Why do we need to store additional set of log coordinates in trx
> record?

Honestly, we do not know what the purpose of that feature is. There is an MTR test in the twitter code for that functionality and it fails if we completely replace it with our CSS implementation. The only place we could find any mention of it is here http://darnaut.blogspot.com/2012/06/changes-in-twitter-mysql-5523t6.html

"Export the last know good binlog position as a status variable from InnoDB
Introduced two new status variables that export the master binary log name and position of a slave as stored by InnoDB. Whenever the SQL thread commits a transaction, InnoDB also commits the master binary log name and position to the system tablespace. Now this information can be retrieved through the Innodb_mysql_master_log_file and Innodb_mysql_master_log_pos status variables.

It is worth noting that the position stored within InnoDB might not match the actual position of the SQL thread. The position is only updated when the SQL thread applies an event that uses an InnoDB table and causes a read-write transaction to be committed. For example, an UPDATE statement which does not change any rows is considered a read-only transaction."

The real problem with it is that is calculates/captures the master pos at a slightly different point than our CSS does and they are not compatible for the purpose of a real crash safe slave. After discussing a bit with Calvin we decided to go with this split implementation just to be safe so that anything that depends on that functionality in the twitter infrastructure will still work.