{"action":"closed","issue":{"url":"https://api.github.com/repos/caetanosauer/zero/issues/19","repository_url":"https://api.github.com/repos/caetanosauer/zero","labels_url":"https://api.github.com/repos/caetanosauer/zero/issues/19/labels{/name}","comments_url":"https://api.github.com/repos/caetanosauer/zero/issues/19/comments","events_url":"https://api.github.com/repos/caetanosauer/zero/issues/19/events","html_url":"https://github.com/caetanosauer/zero/issues/19","id":101939616,"number":19,"title":"Some operations don't set rec_lsn when marking a page dirty","user":{"login":"caetanosauer","id":3465014,"avatar_url":"https://avatars.githubusercontent.com/u/3465014?v=3","gravatar_id":"","url":"https://api.github.com/users/caetanosauer","html_url":"https://github.com/caetanosauer","followers_url":"https://api.github.com/users/caetanosauer/followers","following_url":"https://api.github.com/users/caetanosauer/following{/other_user}","gists_url":"https://api.github.com/users/caetanosauer/gists{/gist_id}","starred_url":"https://api.github.com/users/caetanosauer/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/caetanosauer/subscriptions","organizations_url":"https://api.github.com/users/caetanosauer/orgs","repos_url":"https://api.github.com/users/caetanosauer/repos","events_url":"https://api.github.com/users/caetanosauer/events{/privacy}","received_events_url":"https://api.github.com/users/caetanosauer/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-08-19T16:27:41Z","updated_at":"2016-03-15T20:54:25Z","closed_at":"2016-03-15T20:54:25Z","body":"The rec_lsn field of a frame control block is supposed to indicate the first update which made the page dirty (thus, it is also known as dirtyPageLSN). The goal is to determine a lower bound for the REDO phase of restart, which is the minimum of all rec_lsn collected during a checkpoint.\r\n\r\nThe principle only works if every time a control block is marked as dirty, then its rec_lsn is also updated. This doesn't have to happen atomically, but at least the rec_lsn should always be set before setting the dirty flag (and all CPUs must see the updates in that order, i.e., fences should be used).\r\n\r\n(I'm not referring here to the huge problem that access to control blocks is performed without any synchronization, so even if one flag/field is set before the other, it does not mean that all CPUs will see the updates in that order. So far, we have been counting on the stars aligning correctly, and this is a problem for another bug report.)\r\n\r\nThe problem starts with set_dirty() and update_initial_dirty_lsn() being separate methods. They must always be set together, so why offering separate methods? Indeed, there are some code locations (e.g., in btree \"_ux_*\" methods) which just call set_dirty(). However, I'm not sure this is the actual source of the bug. Where a bug exists for sure is in the method ``xct_t::_update_page_lsns``.\r\n\r\nThere are some cases where this method is called with only a shared latch held on the page (the comments mention that page_evict, i.e., unswizzling and setting the EMLSN, is the only case for now). When this happens, only the page_lsn is updated (using compare and swap) and the rec_lsn is **not** updated. The page is nevertheless marked as dirty. Even if it would update the rec_lsn correctly, this is not done atomically or with proper memory fences, so the bug may still happen.\r\n\r\nBecause the rec_lsn is set as equal to the page_lsn when a page is first loaded (which in my opinion should not be done), this should only cause the \"minor\" problem that a very old LSN will be used as the REDO starting point in a checkpoint. However, old log partitions eventually get recycled as pages are cleaned and older transactions commit. Therefore, it can happen that the min_rec_lsn collected during a checkpoint refers to a log record which does not exist anymore, which not only makes the database unrecoverable in case of a crash, but also causes assertions to fail (e.g., in log_storage::sanity_check).\r\n\r\nThe root cause of this problem is excessive optimization leading to incorrect behavior -- in this case trying to mark a page dirty and update its LSN without an exclusive latch. I didn't think of a proper solution yet -- for now I'm just disabling checkpoints on my experiments."}}