{"action":"created","issue":{"url":"https://api.github.com/repos/druid-io/druid/issues/5692","repository_url":"https://api.github.com/repos/druid-io/druid","labels_url":"https://api.github.com/repos/druid-io/druid/issues/5692/labels{/name}","comments_url":"https://api.github.com/repos/druid-io/druid/issues/5692/comments","events_url":"https://api.github.com/repos/druid-io/druid/issues/5692/events","html_url":"https://github.com/druid-io/druid/pull/5692","id":317309324,"number":5692,"title":"Use unique segment paths for Kafka indexing","user":{"login":"dclim","id":11050882,"avatar_url":"https://avatars2.githubusercontent.com/u/11050882?v=4","gravatar_id":"","url":"https://api.github.com/users/dclim","html_url":"https://github.com/dclim","followers_url":"https://api.github.com/users/dclim/followers","following_url":"https://api.github.com/users/dclim/following{/other_user}","gists_url":"https://api.github.com/users/dclim/gists{/gist_id}","starred_url":"https://api.github.com/users/dclim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dclim/subscriptions","organizations_url":"https://api.github.com/users/dclim/orgs","repos_url":"https://api.github.com/users/dclim/repos","events_url":"https://api.github.com/users/dclim/events{/privacy}","received_events_url":"https://api.github.com/users/dclim/received_events","type":"User","site_admin":false},"labels":[{"id":17287685,"url":"https://api.github.com/repos/druid-io/druid/labels/Bug","name":"Bug","color":"fc2929","default":false}],"state":"open","locked":false,"assignee":{"login":"gianm","id":1214075,"avatar_url":"https://avatars3.githubusercontent.com/u/1214075?v=4","gravatar_id":"","url":"https://api.github.com/users/gianm","html_url":"https://github.com/gianm","followers_url":"https://api.github.com/users/gianm/followers","following_url":"https://api.github.com/users/gianm/following{/other_user}","gists_url":"https://api.github.com/users/gianm/gists{/gist_id}","starred_url":"https://api.github.com/users/gianm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gianm/subscriptions","organizations_url":"https://api.github.com/users/gianm/orgs","repos_url":"https://api.github.com/users/gianm/repos","events_url":"https://api.github.com/users/gianm/events{/privacy}","received_events_url":"https://api.github.com/users/gianm/received_events","type":"User","site_admin":false},"assignees":[{"login":"gianm","id":1214075,"avatar_url":"https://avatars3.githubusercontent.com/u/1214075?v=4","gravatar_id":"","url":"https://api.github.com/users/gianm","html_url":"https://github.com/gianm","followers_url":"https://api.github.com/users/gianm/followers","following_url":"https://api.github.com/users/gianm/following{/other_user}","gists_url":"https://api.github.com/users/gianm/gists{/gist_id}","starred_url":"https://api.github.com/users/gianm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gianm/subscriptions","organizations_url":"https://api.github.com/users/gianm/orgs","repos_url":"https://api.github.com/users/gianm/repos","events_url":"https://api.github.com/users/gianm/events{/privacy}","received_events_url":"https://api.github.com/users/gianm/received_events","type":"User","site_admin":false}],"milestone":null,"comments":5,"created_at":"2018-04-24T16:46:16Z","updated_at":"2018-04-27T06:10:40Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/druid-io/druid/pulls/5692","html_url":"https://github.com/druid-io/druid/pull/5692","diff_url":"https://github.com/druid-io/druid/pull/5692.diff","patch_url":"https://github.com/druid-io/druid/pull/5692.patch"},"body":"#5187 was intended to fix an issue with Kafka indexing service where a task would push segments to deep storage and then fail, and then the subsequent retry task would attempt to push its segment (which contains a different set of offsets from the first task) and publish the metadata, but the push would not happen because the segment data already existed in deep storage. In this scenario, the metadata (most importantly the offset cursors) would not match what's actually in the segment and exactly-once semantics would be violated.\r\n\r\nThis was fixed by supporting file overwriting in deep storage, but it turns out that there are other cases where overwriting the existing segment is undesired behavior. As an example (which may or may not be happening in practice), there could be a task which pushes its segments and publishes metadata but then fails before it can notify the supervisor that it is done. If the supervisor then creates a retry task to attempt again, the task will eventually fail when the transactional metadata commit fails, but by this point it will have already pushed its version of the segments into deep storage, overwriting the good set of segments there. This will lead to a similar situation as the previous case, where the same segment ID could have different data on historicals and in deep storage, and where Kafka messages can be duplicated or missed.\r\n\r\nThe cleanest way to handle all these cases is to have each Kafka task write its segments to a separate location in deep storage so that the metadata published definitely matches the segment set and there's no interaction between tasks. If a task pushes and then the metadata publish fails, it will attempt to clean up the orphaned segments, but it is expected that in some unhandled exception cases, the segments may remain in deep storage and may need to be cleaned up manually."},"comment":{"url":"https://api.github.com/repos/druid-io/druid/issues/comments/384874233","html_url":"https://github.com/druid-io/druid/pull/5692#issuecomment-384874233","issue_url":"https://api.github.com/repos/druid-io/druid/issues/5692","id":384874233,"user":{"login":"dclim","id":11050882,"avatar_url":"https://avatars2.githubusercontent.com/u/11050882?v=4","gravatar_id":"","url":"https://api.github.com/users/dclim","html_url":"https://github.com/dclim","followers_url":"https://api.github.com/users/dclim/followers","following_url":"https://api.github.com/users/dclim/following{/other_user}","gists_url":"https://api.github.com/users/dclim/gists{/gist_id}","starred_url":"https://api.github.com/users/dclim/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dclim/subscriptions","organizations_url":"https://api.github.com/users/dclim/orgs","repos_url":"https://api.github.com/users/dclim/repos","events_url":"https://api.github.com/users/dclim/events{/privacy}","received_events_url":"https://api.github.com/users/dclim/received_events","type":"User","site_admin":false},"created_at":"2018-04-27T06:10:40Z","updated_at":"2018-04-27T06:10:40Z","author_association":"MEMBER","body":"@gianm changes made, thanks for the review guys. Good catch on killing segments at \"Our segments really do exist, awaiting handoff\", I meant to do that but missed it and only caught the exception case.\r\n\r\n@b-slim that seems reasonable to me; I think it makes sense though to wait and see if anyone complains about this; I think the cases that it will leave garbage are actually quite small, and not much higher than the current implementation where tasks overwrite each other's segments."}}
{"actor":{"display_login":"dclim"}}