)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1004059,"name":"Paolo Cocchi","email":"paolo.cocchi@couchbase.com","username":"paolococchi","avatars":[{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"22daa95c5066e7a3062efcf9aa49b81d0cbe766b","unresolved":true,"context_lines":[{"line_number":31,"context_line":"        #6 MutationCommandContext::step() kv_engine/daemon/protocol/mcbp/mutation_context.cc:54 (memcached+0x7385f7)"},{"line_number":32,"context_line":"        ..."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"When setEngineStorage is called from"},{"line_number":35,"context_line":"EventuallyPersistentEngine::store_if (when issueing a SyncWrite) from"},{"line_number":36,"context_line":"the frontend thread, the per-frontend thread mutex is held when"},{"line_number":37,"context_line":"modifying Cookie:engine_storage. However when the background thread"},{"line_number":38,"context_line":"later modifies Cookie:engine_storage the front-end thread mutex is not"},{"line_number":39,"context_line":"held."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Address this by making Cookie::engine_storage atomic. We could have"},{"line_number":42,"context_line":"added a mutex around it, but that would add additional space"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5d98aecc_666d1389","line":39,"range":{"start_line":34,"start_character":0,"end_line":39,"end_character":5},"updated":"2021-10-11 04:05:42.000000000","message":"There\u0027s something that I don\u0027t get here.\n\nThe connection is paused after the SW is stored for processing, and then woken up only by:\n\nSyncWriteCompleteCallback KVBucket::makeSyncWriteCompleteCB() {\n    return [\u0026engine \u003d this-\u003eengine](const CookieIface* cookie,\n                                    cb::engine_errc status) {\n        if (status !\u003d cb::engine_errc::success) {\n            // For non-success status codes clear the cookie\u0027s engine_specific;\n            // as the operation is now complete. This ensures that any\n            // subsequent call by the same cookie to store() is treated as a new\n            // operation (and not the completion of the previous one).\n            engine.storeEngineSpecific(cookie, nullptr);\n        }\n        engine.notifyIOComplete(cookie, status);\n    };\n}\n\nSo, the Cookie::engine_specific is written in the bg-thread only in a state where the fe-thread can\u0027t really run on the same Cookie yet, as the notification is the latest step in SyncWriteCompleteCallback.\n\nIs TSAN spotting more a \"potential\" race than an actual one.. ? Or maybe the test is executing some Out Of Order on that connection.. ?\nI don\u0027t recall now if any OOO can really happen when SW is in the mix..","commit_id":"a1028bd527b8b6170260914fbf7aead4af91740d"},{"author":{"_account_id":1004059,"name":"Paolo Cocchi","email":"paolo.cocchi@couchbase.com","username":"paolococchi","avatars":[{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/3367cfbc4fbf29e6d6b3a5397e41849d.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"e7a4897a67ead6410bfa4d7168e437621118097d","unresolved":true,"context_lines":[{"line_number":31,"context_line":"        #6 MutationCommandContext::step() kv_engine/daemon/protocol/mcbp/mutation_context.cc:54 (memcached+0x7385f7)"},{"line_number":32,"context_line":"        ..."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"When setEngineStorage is called from"},{"line_number":35,"context_line":"EventuallyPersistentEngine::store_if (when issueing a SyncWrite) from"},{"line_number":36,"context_line":"the frontend thread, the per-frontend thread mutex is held when"},{"line_number":37,"context_line":"modifying Cookie:engine_storage. However when the background thread"},{"line_number":38,"context_line":"later modifies Cookie:engine_storage the front-end thread mutex is not"},{"line_number":39,"context_line":"held."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Address this by making Cookie::engine_storage atomic. We could have"},{"line_number":42,"context_line":"added a mutex around it, but that would add additional space"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"26997ab0_b07fa1eb","line":39,"range":{"start_line":34,"start_character":0,"end_line":39,"end_character":5},"in_reply_to":"372f4f05_50b4c2eb","updated":"2021-10-11 09:07:55.000000000","message":"Got it, thanks","commit_id":"a1028bd527b8b6170260914fbf7aead4af91740d"},{"author":{"_account_id":1000966,"name":"Dave Rigby","email":"daver@couchbase.com","username":"drigby","avatars":[{"url":"https://www.gravatar.com/avatar/514e75a8d75cc1fcdb22433d445ae8f1.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/514e75a8d75cc1fcdb22433d445ae8f1.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/514e75a8d75cc1fcdb22433d445ae8f1.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/514e75a8d75cc1fcdb22433d445ae8f1.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"c8ee04a54798db7d7ff135d4e554a7f83a65eea3","unresolved":true,"context_lines":[{"line_number":31,"context_line":"        #6 MutationCommandContext::step() kv_engine/daemon/protocol/mcbp/mutation_context.cc:54 (memcached+0x7385f7)"},{"line_number":32,"context_line":"        ..."},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"When setEngineStorage is called from"},{"line_number":35,"context_line":"EventuallyPersistentEngine::store_if (when issueing a SyncWrite) from"},{"line_number":36,"context_line":"the frontend thread, the per-frontend thread mutex is held when"},{"line_number":37,"context_line":"modifying Cookie:engine_storage. However when the background thread"},{"line_number":38,"context_line":"later modifies Cookie:engine_storage the front-end thread mutex is not"},{"line_number":39,"context_line":"held."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"Address this by making Cookie::engine_storage atomic. We could have"},{"line_number":42,"context_line":"added a mutex around it, but that would add additional space"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"372f4f05_50b4c2eb","line":39,"range":{"start_line":34,"start_character":0,"end_line":39,"end_character":5},"in_reply_to":"5d98aecc_666d1389","updated":"2021-10-11 08:18:18.000000000","message":"\u003e So, the Cookie::engine_specific is written in the bg-thread only in a state where the fe-thread can\u0027t really run on the same Cookie yet, as the notification is the latest step in SyncWriteCompleteCallback.\n\nCorrect, in this particular scenario the FE thread (well, Cookie) is in ewouldblock state and hence shouldn\u0027t (by our application logic) run while the cookie is being updated - so in one sense the Cookie::engine_storage should not be accessed at the same time.\n\n_However_, if we write to Cookie::engine_storage without a lock (or atomic), then it\u0027s possible that the write by NonIO thread will not be observed by the FE thread when it next runs.","commit_id":"a1028bd527b8b6170260914fbf7aead4af91740d"}]}
