{"action":"created","issue":{"url":"https://api.github.com/repos/scylladb/scylla/issues/1775","repository_url":"https://api.github.com/repos/scylladb/scylla","labels_url":"https://api.github.com/repos/scylladb/scylla/issues/1775/labels{/name}","comments_url":"https://api.github.com/repos/scylladb/scylla/issues/1775/comments","events_url":"https://api.github.com/repos/scylladb/scylla/issues/1775/events","html_url":"https://github.com/scylladb/scylla/issues/1775","id":184452867,"number":1775,"title":"is this a bug when commitlog replay?","user":{"login":"duanshisan","id":16314600,"avatar_url":"https://avatars.githubusercontent.com/u/16314600?v=3","gravatar_id":"","url":"https://api.github.com/users/duanshisan","html_url":"https://github.com/duanshisan","followers_url":"https://api.github.com/users/duanshisan/followers","following_url":"https://api.github.com/users/duanshisan/following{/other_user}","gists_url":"https://api.github.com/users/duanshisan/gists{/gist_id}","starred_url":"https://api.github.com/users/duanshisan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/duanshisan/subscriptions","organizations_url":"https://api.github.com/users/duanshisan/orgs","repos_url":"https://api.github.com/users/duanshisan/repos","events_url":"https://api.github.com/users/duanshisan/events{/privacy}","received_events_url":"https://api.github.com/users/duanshisan/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-10-21T10:30:14Z","updated_at":"2016-10-25T10:22:44Z","closed_at":null,"body":"**when replay in function 'db::commitlog::read_log_file(...)',  if the code 'return s.produce(buf.share(0, data_size), rp);' has error, this function wait  future will be set exception() in the below code (seastar/core/stream.hh):**\r\n\r\ntemplate <typename... T>\r\ninline\r\nfuture<>\r\nstream<T...>::produce(T... data) {\r\n    auto ret = futurize<void>::apply(_sub->_next, std::move(data)...);\r\n    if (ret.available() && !ret.failed()) {\r\n        // Native network stack depends on stream::produce() returning\r\n        // a ready future to push packets along without dropping.  As\r\n        // a temporary workaround, special case a ready, unfailed future\r\n        // and return it immediately, so that then_wrapped(), below,\r\n        // doesn't convert a ready future to an unready one.\r\n        return ret;\r\n    }\r\n    return ret.then_wrapped([this] (auto&& f) {\r\n        try {\r\n            f.get();\r\n        } catch (...) {\r\n            _done.set_exception(std::current_exception());  **//set exception first**\r\n            // FIXME: tell the producer to stop producing\r\n            throw;\r\n        }\r\n    });\r\n}\r\n\r\n**but ,this exception be throwed again, will be catched by bellow code (db/commitlog/commitlog.cc):**\r\n\r\nsubscription<temporary_buffer<char>, db::replay_position>\r\ndb::commitlog::read_log_file(file f, commit_load_reader_func next, position_type off) {\r\n   .....\r\n    w->s.started().then(std::bind(&work::read_file, w.get())).then([w] {\r\n        w->s.close();\r\n    }).handle_exception([w](auto ep) {\r\n        w->s.set_exception(ep);   **//here set exception again**\r\n    });\r\n\r\n    return ret;\r\n}\r\n\r\n**so set exception  twice will cause error in bellow code (seastar/core/future.hh):**\r\n\r\nvoid set_exception(std::exception_ptr ex) noexcept {\r\n        assert(_state == state::future);    **//assert error**\r\n        new (&_u.ex) std::exception_ptr(ex);\r\n        _state = state::exception;\r\n}\r\n"},"comment":{"url":"https://api.github.com/repos/scylladb/scylla/issues/comments/255996822","html_url":"https://github.com/scylladb/scylla/issues/1775#issuecomment-255996822","issue_url":"https://api.github.com/repos/scylladb/scylla/issues/1775","id":255996822,"user":{"login":"duanshisan","id":16314600,"avatar_url":"https://avatars.githubusercontent.com/u/16314600?v=3","gravatar_id":"","url":"https://api.github.com/users/duanshisan","html_url":"https://github.com/duanshisan","followers_url":"https://api.github.com/users/duanshisan/followers","following_url":"https://api.github.com/users/duanshisan/following{/other_user}","gists_url":"https://api.github.com/users/duanshisan/gists{/gist_id}","starred_url":"https://api.github.com/users/duanshisan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/duanshisan/subscriptions","organizations_url":"https://api.github.com/users/duanshisan/orgs","repos_url":"https://api.github.com/users/duanshisan/repos","events_url":"https://api.github.com/users/duanshisan/events{/privacy}","received_events_url":"https://api.github.com/users/duanshisan/received_events","type":"User","site_admin":false},"created_at":"2016-10-25T10:22:44Z","updated_at":"2016-10-25T10:22:44Z","body":"@glommer \r\n\r\nOnly for this commitlog replay problem, whether to do a simple modification like this:\r\n\r\ndb/commitlog/commitlog.cc\r\n\r\nsubscription<temporary_buffer<char>, db::replay_position>\r\nhive_commitlog::read_log_file(file f, commit_load_reader_func next, position_type off) {\r\n\r\n    struct work {\r\n        ...\r\n        **bool already_set_exception = false; //add a flag** \r\n\r\n        future<> read_entry() {\r\n        ...\r\n              return s.produce(buf.share(0, data_size), rp).**then_wrapped([this](auto f)mutable{\r\n                        try{\r\n                            f.get();\r\n                            return make_ready_future<>();\r\n                        }catch(...){\r\n                            already_set_exception = true;\r\n                            throw;\r\n                        }\r\n              });**\r\n        ...\r\n        }\r\n    };\r\n\r\n    auto w = make_lw_shared<work>(std::move(f), off);\r\n    auto ret = w->s.listen(std::move(next));\r\n    w->s.started().then(std::bind(&work::read_file, w.get())).then([w] {\r\n        w->s.close();\r\n    }).handle_exception([w](auto ep) {\r\n        **if(!w->already_set_exception){\r\n            w->s.set_exception(ep);\r\n        }**\r\n    });\r\n\r\n}//end of read_log_file\r\n\r\n\r\n**use a flag can solve this problem but looks very uncomfortable, whether have other better solution? and can we modify the promise class when set_value or set_exception don't assert directly,  instead by other better way?**\r\n\r\n\r\n"}}
{"actor":{"display_login":"duanshisan"}}