)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1004368,"name":"Richard de Mellow","email":"richard.demellow@couchbase.com","username":"rdemellow","avatars":[{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"b460d30c4a6fd154f690d89a18bbc9246968c796","unresolved":true,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Previously, StatCheckpointTask and StatDCPTask immediately wrote"},{"line_number":10,"context_line":"responses when collecting stats while on a background thread."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"TSAN reported this as unsafe; no locks prevent potential racing with a"},{"line_number":13,"context_line":"frontend thread manipulating the cookie."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change both tasks to accumulate task values, but leave the frontend"},{"line_number":16,"context_line":"thread to actually write the responses when it resumes the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"f3a972d8_ce90f605","line":13,"range":{"start_line":11,"start_character":0,"end_line":13,"end_character":40},"updated":"2021-10-20 10:50:09.000000000","message":"Worth putting the TSAN report in the description of the commit for prosperity","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"9828df35696fc7e342e217f7fd6bebcbfac01b97","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Previously, StatCheckpointTask and StatDCPTask immediately wrote"},{"line_number":10,"context_line":"responses when collecting stats while on a background thread."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"TSAN reported this as unsafe; no locks prevent potential racing with a"},{"line_number":13,"context_line":"frontend thread manipulating the cookie."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Change both tasks to accumulate task values, but leave the frontend"},{"line_number":16,"context_line":"thread to actually write the responses when it resumes the"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"925cf247_f403e4dd","line":13,"range":{"start_line":11,"start_character":0,"end_line":13,"end_character":40},"in_reply_to":"f3a972d8_ce90f605","updated":"2021-10-20 12:00:58.000000000","message":"Done","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"}],"engines/ep/src/background_stat_task.h":[{"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":"588af3e689329291792cdf3e25d6b4fc923d3ed0","unresolved":true,"context_lines":[{"line_number":70,"context_line":"     */"},{"line_number":71,"context_line":"    AddStatFn getDeferredAddStat();"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    EventuallyPersistentEngine* e;"},{"line_number":74,"context_line":"    const CookieIface* cookie;"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    // stats which have been collected while running in a background task"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"12aa5f67_9d963561","line":73,"updated":"2021-11-02 12:12:59.000000000","message":"Prefer reference if EPEngine must be non-null.","commit_id":"355b333d09f3fd32bc579b2d72ee59fc758f5435"},{"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":"588af3e689329291792cdf3e25d6b4fc923d3ed0","unresolved":true,"context_lines":[{"line_number":71,"context_line":"    AddStatFn getDeferredAddStat();"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    EventuallyPersistentEngine* e;"},{"line_number":74,"context_line":"    const CookieIface* cookie;"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    // stats which have been collected while running in a background task"},{"line_number":77,"context_line":"    std::vector\u003cstd::pair\u003cstd::string, std::string\u003e\u003e stats;"}],"source_content_type":"text/x-csrc","patch_set":13,"id":"29b8aa8f_700f4ceb","line":74,"updated":"2021-11-02 12:12:59.000000000","message":"Note: You could forward-declare these types; but given this file is only included from one place which already pulls in the definitions of these it doesn\u0027t really matter...","commit_id":"355b333d09f3fd32bc579b2d72ee59fc758f5435"}],"engines/ep/src/ep_engine.cc":[{"author":{"_account_id":1004368,"name":"Richard de Mellow","email":"richard.demellow@couchbase.com","username":"rdemellow","avatars":[{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/35c97da872c07458f5e5bc4938b2489f.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"b460d30c4a6fd154f690d89a18bbc9246968c796","unresolved":true,"context_lines":[{"line_number":1854,"context_line":"    return cookie-\u003egetEngineStorage();"},{"line_number":1855,"context_line":"}"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"/**"},{"line_number":1858,"context_line":" * Small helper which owns a shared ptr to a stat task."},{"line_number":1859,"context_line":" *"},{"line_number":1860,"context_line":" * Once allocated, a pointer to TaskWrapper may be stored as the engine specific"},{"line_number":1861,"context_line":" * data, to be retrieved when the frontend thread is notified. Intended to be"},{"line_number":1862,"context_line":" * allocated and destroyed only by {store,retrieve}StatTask."},{"line_number":1863,"context_line":" *"},{"line_number":1864,"context_line":" * Not strictly required, but improves readability over direct pointers to"},{"line_number":1865,"context_line":" * shared pointers."},{"line_number":1866,"context_line":" */"},{"line_number":1867,"context_line":"struct TaskWrapper {"},{"line_number":1868,"context_line":"    // constructor needed to allow make_unique (until C++20)"},{"line_number":1869,"context_line":"    TaskWrapper(std::shared_ptr\u003cBackgroundStatTask\u003e task)"},{"line_number":1870,"context_line":"        : task(std::move(task)) {"},{"line_number":1871,"context_line":"    }"},{"line_number":1872,"context_line":"    std::shared_ptr\u003cBackgroundStatTask\u003e task;"},{"line_number":1873,"context_line":"};"},{"line_number":1874,"context_line":""},{"line_number":1875,"context_line":"void EventuallyPersistentEngine::storeStatTask("},{"line_number":1876,"context_line":"        const CookieIface* cookie, std::shared_ptr\u003cBackgroundStatTask\u003e task) {"}],"source_content_type":"text/x-c","patch_set":8,"id":"fedcc9aa_e7b1bc87","line":1873,"range":{"start_line":1857,"start_character":0,"end_line":1873,"end_character":2},"updated":"2021-10-20 10:50:09.000000000","message":"Not really sure this does improve the readability, kind of just adds an extra layer of abstraction with not much gain and more code to maintain.","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"9828df35696fc7e342e217f7fd6bebcbfac01b97","unresolved":false,"context_lines":[{"line_number":1854,"context_line":"    return cookie-\u003egetEngineStorage();"},{"line_number":1855,"context_line":"}"},{"line_number":1856,"context_line":""},{"line_number":1857,"context_line":"/**"},{"line_number":1858,"context_line":" * Small helper which owns a shared ptr to a stat task."},{"line_number":1859,"context_line":" *"},{"line_number":1860,"context_line":" * Once allocated, a pointer to TaskWrapper may be stored as the engine specific"},{"line_number":1861,"context_line":" * data, to be retrieved when the frontend thread is notified. Intended to be"},{"line_number":1862,"context_line":" * allocated and destroyed only by {store,retrieve}StatTask."},{"line_number":1863,"context_line":" *"},{"line_number":1864,"context_line":" * Not strictly required, but improves readability over direct pointers to"},{"line_number":1865,"context_line":" * shared pointers."},{"line_number":1866,"context_line":" */"},{"line_number":1867,"context_line":"struct TaskWrapper {"},{"line_number":1868,"context_line":"    // constructor needed to allow make_unique (until C++20)"},{"line_number":1869,"context_line":"    TaskWrapper(std::shared_ptr\u003cBackgroundStatTask\u003e task)"},{"line_number":1870,"context_line":"        : task(std::move(task)) {"},{"line_number":1871,"context_line":"    }"},{"line_number":1872,"context_line":"    std::shared_ptr\u003cBackgroundStatTask\u003e task;"},{"line_number":1873,"context_line":"};"},{"line_number":1874,"context_line":""},{"line_number":1875,"context_line":"void EventuallyPersistentEngine::storeStatTask("},{"line_number":1876,"context_line":"        const CookieIface* cookie, std::shared_ptr\u003cBackgroundStatTask\u003e task) {"}],"source_content_type":"text/x-c","patch_set":8,"id":"95e46b65_7dd376cc","line":1873,"range":{"start_line":1857,"start_character":0,"end_line":1873,"end_character":2},"in_reply_to":"fedcc9aa_e7b1bc87","updated":"2021-10-20 12:00:58.000000000","message":"Done","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"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":"6207c9fb081c5fab5c2aea6418a4e20a3e11f820","unresolved":true,"context_lines":[{"line_number":1878,"context_line":"    // Once the frontend thread is notified, it must call retrieveStatTask,"},{"line_number":1879,"context_line":"    // which will free the wrapper."},{"line_number":1880,"context_line":"    auto wrapper \u003d std::make_unique\u003cTaskWrapper\u003e(task);"},{"line_number":1881,"context_line":"    storeEngineSpecific(cookie, wrapper.get());"},{"line_number":1882,"context_line":"    wrapper.release();"},{"line_number":1883,"context_line":"}"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-c","patch_set":8,"id":"36fea946_eb89d93c","line":1881,"updated":"2021-10-20 10:53:24.000000000","message":"Nit: Exception safer if you use wrapper.release() here (it returns the owned ptr).","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"1b05799cd128acf61b1a3ced7852de6fbf71bc22","unresolved":true,"context_lines":[{"line_number":1878,"context_line":"    // Once the frontend thread is notified, it must call retrieveStatTask,"},{"line_number":1879,"context_line":"    // which will free the wrapper."},{"line_number":1880,"context_line":"    auto wrapper \u003d std::make_unique\u003cTaskWrapper\u003e(task);"},{"line_number":1881,"context_line":"    storeEngineSpecific(cookie, wrapper.get());"},{"line_number":1882,"context_line":"    wrapper.release();"},{"line_number":1883,"context_line":"}"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-c","patch_set":8,"id":"dda6b1db_23aed526","line":1881,"in_reply_to":"36fea946_eb89d93c","updated":"2021-10-20 11:26:20.000000000","message":"Hmmm - I think I made different assumptions about the state if storeEngineSpecific were to throw.\n\nKeeping it in the unique_ptr until after the call means we won\u0027t leak the allocated object if the call throws, but that relies on storeEngineSpecific to _guarantee_ the provided ptr isn\u0027t stored (or at least, isn\u0027t ever _used_) if it throws - that seems reasonable if the exception will always kill the connection. wrapper.release() here in that situation would leave no one to destroy the object (memcached side definitely won\u0027t).\n\nOTOH, if the throw did _not_ prevent the connection coming back round to hit retrieveStatTask with a dangling ptr then indeed it\u0027s going to heap-use-after-free, and release() would be the logical thing to do - we\u0027d be confident that the object will definitely be freed later.\n\nhappy to .release() if that\u0027s a better fit for the expected situations :salute:","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"9828df35696fc7e342e217f7fd6bebcbfac01b97","unresolved":false,"context_lines":[{"line_number":1878,"context_line":"    // Once the frontend thread is notified, it must call retrieveStatTask,"},{"line_number":1879,"context_line":"    // which will free the wrapper."},{"line_number":1880,"context_line":"    auto wrapper \u003d std::make_unique\u003cTaskWrapper\u003e(task);"},{"line_number":1881,"context_line":"    storeEngineSpecific(cookie, wrapper.get());"},{"line_number":1882,"context_line":"    wrapper.release();"},{"line_number":1883,"context_line":"}"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-c","patch_set":8,"id":"ed305bb8_2abdc132","line":1881,"in_reply_to":"98a6f56f_cab1253c","updated":"2021-10-20 12:00:58.000000000","message":"Done","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"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":"9f66168d5dc470459ed5f7b92d2cde33b99975e3","unresolved":true,"context_lines":[{"line_number":1878,"context_line":"    // Once the frontend thread is notified, it must call retrieveStatTask,"},{"line_number":1879,"context_line":"    // which will free the wrapper."},{"line_number":1880,"context_line":"    auto wrapper \u003d std::make_unique\u003cTaskWrapper\u003e(task);"},{"line_number":1881,"context_line":"    storeEngineSpecific(cookie, wrapper.get());"},{"line_number":1882,"context_line":"    wrapper.release();"},{"line_number":1883,"context_line":"}"},{"line_number":1884,"context_line":""}],"source_content_type":"text/x-c","patch_set":8,"id":"98a6f56f_cab1253c","line":1881,"in_reply_to":"dda6b1db_23aed526","updated":"2021-10-20 11:30:36.000000000","message":"That\u0027s probably fair - exception-safety in this situation is somewhat meaningless given we are just storing a raw pointer into the API. I was mostly thinking in the general case.\n\nOne other route you could take (long the lines of Richard\u0027s comment) would be to just \"fix\" this issue in the simple way (don\u0027t address the potential ownership issues), and look into that in a follow-up patch via std::any or a common base class for all stored engine_specific \"things\"...","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"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":"6207c9fb081c5fab5c2aea6418a4e20a3e11f820","unresolved":true,"context_lines":[{"line_number":3812,"context_line":"    AddStatFn add_stat;"},{"line_number":3813,"context_line":"};"},{"line_number":3814,"context_line":""},{"line_number":3815,"context_line":"class BackgroundStatTask : public GlobalTask {"},{"line_number":3816,"context_line":"public:"},{"line_number":3817,"context_line":"    using Callback \u003d std::function\u003ccb::engine_errc(EventuallyPersistentEngine*,"},{"line_number":3818,"context_line":"                                                   const AddStatFn\u0026)\u003e;"}],"source_content_type":"text/x-c","patch_set":8,"id":"9ff3ae6a_abbaa702","line":3815,"updated":"2021-10-20 10:53:24.000000000","message":"Please document this class.","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"9828df35696fc7e342e217f7fd6bebcbfac01b97","unresolved":false,"context_lines":[{"line_number":3812,"context_line":"    AddStatFn add_stat;"},{"line_number":3813,"context_line":"};"},{"line_number":3814,"context_line":""},{"line_number":3815,"context_line":"class BackgroundStatTask : public GlobalTask {"},{"line_number":3816,"context_line":"public:"},{"line_number":3817,"context_line":"    using Callback \u003d std::function\u003ccb::engine_errc(EventuallyPersistentEngine*,"},{"line_number":3818,"context_line":"                                                   const AddStatFn\u0026)\u003e;"}],"source_content_type":"text/x-c","patch_set":8,"id":"25bc581d_29c95be1","line":3815,"in_reply_to":"9ff3ae6a_abbaa702","updated":"2021-10-20 12:00:58.000000000","message":"Done","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"fc9ae4953b0c4210defea85f4c064c1808e2c842","unresolved":true,"context_lines":[{"line_number":3840,"context_line":"                    e.what(),"},{"line_number":3841,"context_line":"                    getDescription());"},{"line_number":3842,"context_line":"        }"},{"line_number":3843,"context_line":"        e-\u003enotifyIOComplete(cookie, result);"},{"line_number":3844,"context_line":"        return false;"},{"line_number":3845,"context_line":"    }"},{"line_number":3846,"context_line":""}],"source_content_type":"text/x-c","patch_set":8,"id":"e7c9b697_56529213","line":3843,"range":{"start_line":3843,"start_character":8,"end_line":3843,"end_character":44},"updated":"2021-10-20 12:30:05.000000000","message":"Thinking about it, returning non-success will make SteppableCommandContext::drive() skip calling back in and we\u0027d leave the engine specific set (vaguely seems similar to the aborted sync write case). Can either make this always success and but store the actual result for when the frontend thread writes the response, or clear the engine specific here. Clearing it here would be consistent with aborted sync writes (IIRC).\n\nAlso probably a hint that I need to add tests with failing stats calls to actually cover this case","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"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":"24c8c47f54f368c7efd8d4dfa61d79a50f7b65d7","unresolved":true,"context_lines":[{"line_number":3840,"context_line":"                    e.what(),"},{"line_number":3841,"context_line":"                    getDescription());"},{"line_number":3842,"context_line":"        }"},{"line_number":3843,"context_line":"        e-\u003enotifyIOComplete(cookie, result);"},{"line_number":3844,"context_line":"        return false;"},{"line_number":3845,"context_line":"    }"},{"line_number":3846,"context_line":""}],"source_content_type":"text/x-c","patch_set":8,"id":"6bf5e87e_87f5c085","line":3843,"range":{"start_line":3843,"start_character":8,"end_line":3843,"end_character":44},"in_reply_to":"e7c9b697_56529213","updated":"2021-10-20 12:56:00.000000000","message":"Yeah, this is probably a sign that we should work on the engine_specific face - it\u0027s not really designed to be owning at the moment; or at least nothing is cleaned up. For example the cookie should probably destroy the engine_specific one way or another when a cookie finishes execution, regardless of status.","commit_id":"0b9b169cf3958549512b0b432a76bc38449c5972"},{"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":"1f500af360210ee1bf28cb53c3e1d9df20836f7c","unresolved":true,"context_lines":[{"line_number":3822,"context_line":"     * Do potentially expensive work to collect stats in a background task."},{"line_number":3823,"context_line":"     * @return status of operation"},{"line_number":3824,"context_line":"     */"},{"line_number":3825,"context_line":"    virtual cb::engine_errc collectStats() \u003d 0;"},{"line_number":3826,"context_line":""},{"line_number":3827,"context_line":"    bool run() override {"},{"line_number":3828,"context_line":"        TRACE_EVENT0(\"ep-engine/task\", \"BackgroundStatTask\");"}],"source_content_type":"text/x-c","patch_set":11,"id":"fb780f99_e628994b","line":3825,"updated":"2021-10-21 12:16:12.000000000","message":"Does this need to be public?","commit_id":"6d0e0ae156a4611b088516549792723bc6204644"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f741d17755138696663548d33c3167db2faf16f3","unresolved":false,"context_lines":[{"line_number":3822,"context_line":"     * Do potentially expensive work to collect stats in a background task."},{"line_number":3823,"context_line":"     * @return status of operation"},{"line_number":3824,"context_line":"     */"},{"line_number":3825,"context_line":"    virtual cb::engine_errc collectStats() \u003d 0;"},{"line_number":3826,"context_line":""},{"line_number":3827,"context_line":"    bool run() override {"},{"line_number":3828,"context_line":"        TRACE_EVENT0(\"ep-engine/task\", \"BackgroundStatTask\");"}],"source_content_type":"text/x-c","patch_set":11,"id":"4ab0ad86_fd98b11b","line":3825,"in_reply_to":"fb780f99_e628994b","updated":"2021-10-21 12:33:08.000000000","message":"Done","commit_id":"6d0e0ae156a4611b088516549792723bc6204644"},{"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":"1f500af360210ee1bf28cb53c3e1d9df20836f7c","unresolved":true,"context_lines":[{"line_number":3851,"context_line":"     * task as doing so could be racy. Instead, store them for when the"},{"line_number":3852,"context_line":"     * frontend thread revisits this operation."},{"line_number":3853,"context_line":"     */"},{"line_number":3854,"context_line":"    AddStatFn getDeferredAddStat() {"},{"line_number":3855,"context_line":"        return [this](std::string_view key,"},{"line_number":3856,"context_line":"                      std::string_view value,"},{"line_number":3857,"context_line":"                      const void*) { this-\u003estats.emplace_back(key, value); };"}],"source_content_type":"text/x-c","patch_set":11,"id":"c3782335_4e6ef78f","line":3854,"updated":"2021-10-21 12:16:12.000000000","message":"Does this need to be public?","commit_id":"6d0e0ae156a4611b088516549792723bc6204644"},{"author":{"_account_id":1003828,"name":"James H","email":"james.harrison@couchbase.com","username":"jameseh96","avatars":[{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/70d2875f461e17e71a2a16972681c794.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"f741d17755138696663548d33c3167db2faf16f3","unresolved":false,"context_lines":[{"line_number":3851,"context_line":"     * task as doing so could be racy. Instead, store them for when the"},{"line_number":3852,"context_line":"     * frontend thread revisits this operation."},{"line_number":3853,"context_line":"     */"},{"line_number":3854,"context_line":"    AddStatFn getDeferredAddStat() {"},{"line_number":3855,"context_line":"        return [this](std::string_view key,"},{"line_number":3856,"context_line":"                      std::string_view value,"},{"line_number":3857,"context_line":"                      const void*) { this-\u003estats.emplace_back(key, value); };"}],"source_content_type":"text/x-c","patch_set":11,"id":"accab30f_aebd9da0","line":3854,"in_reply_to":"c3782335_4e6ef78f","updated":"2021-10-21 12:33:08.000000000","message":"Done","commit_id":"6d0e0ae156a4611b088516549792723bc6204644"},{"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":"db095750e359b7e06f000e11b12a6be9e889c9a2","unresolved":true,"context_lines":[{"line_number":3797,"context_line":"    AddStatFn add_stat;"},{"line_number":3798,"context_line":"};"},{"line_number":3799,"context_line":""},{"line_number":3800,"context_line":"/**"},{"line_number":3801,"context_line":" * Base type for tasks which gather stats on a background task."},{"line_number":3802,"context_line":" *"},{"line_number":3803,"context_line":" * For use when generating stats is likely to be expensive, to avoid"}],"source_content_type":"text/x-c","patch_set":12,"id":"c31ecd67_8d507be0","line":3800,"updated":"2021-11-01 16:21:50.000000000","message":"Suggest pulling the BackgroundStatTask code into it\u0027s own file - ep_engine.cc is already over 7000 lines...","commit_id":"e56d1048099fc6ad81a72b03df3d3725a79e78de"}]}
