{"action":"created","issue":{"url":"https://api.github.com/repos/pravega/pravega/issues/2539","repository_url":"https://api.github.com/repos/pravega/pravega","labels_url":"https://api.github.com/repos/pravega/pravega/issues/2539/labels{/name}","comments_url":"https://api.github.com/repos/pravega/pravega/issues/2539/comments","events_url":"https://api.github.com/repos/pravega/pravega/issues/2539/events","html_url":"https://github.com/pravega/pravega/pull/2539","id":319728755,"number":2539,"title":"Issue 2538: (SegmentStore) Do not auto-delete Attribute Segment if main Segment is deleted","user":{"login":"andreipaduroiu","id":14354377,"avatar_url":"https://avatars2.githubusercontent.com/u/14354377?v=4","gravatar_id":"","url":"https://api.github.com/users/andreipaduroiu","html_url":"https://github.com/andreipaduroiu","followers_url":"https://api.github.com/users/andreipaduroiu/followers","following_url":"https://api.github.com/users/andreipaduroiu/following{/other_user}","gists_url":"https://api.github.com/users/andreipaduroiu/gists{/gist_id}","starred_url":"https://api.github.com/users/andreipaduroiu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/andreipaduroiu/subscriptions","organizations_url":"https://api.github.com/users/andreipaduroiu/orgs","repos_url":"https://api.github.com/users/andreipaduroiu/repos","events_url":"https://api.github.com/users/andreipaduroiu/events{/privacy}","received_events_url":"https://api.github.com/users/andreipaduroiu/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"fpj","id":572920,"avatar_url":"https://avatars3.githubusercontent.com/u/572920?v=4","gravatar_id":"","url":"https://api.github.com/users/fpj","html_url":"https://github.com/fpj","followers_url":"https://api.github.com/users/fpj/followers","following_url":"https://api.github.com/users/fpj/following{/other_user}","gists_url":"https://api.github.com/users/fpj/gists{/gist_id}","starred_url":"https://api.github.com/users/fpj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fpj/subscriptions","organizations_url":"https://api.github.com/users/fpj/orgs","repos_url":"https://api.github.com/users/fpj/repos","events_url":"https://api.github.com/users/fpj/events{/privacy}","received_events_url":"https://api.github.com/users/fpj/received_events","type":"User","site_admin":false},"assignees":[{"login":"fpj","id":572920,"avatar_url":"https://avatars3.githubusercontent.com/u/572920?v=4","gravatar_id":"","url":"https://api.github.com/users/fpj","html_url":"https://github.com/fpj","followers_url":"https://api.github.com/users/fpj/followers","following_url":"https://api.github.com/users/fpj/following{/other_user}","gists_url":"https://api.github.com/users/fpj/gists{/gist_id}","starred_url":"https://api.github.com/users/fpj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fpj/subscriptions","organizations_url":"https://api.github.com/users/fpj/orgs","repos_url":"https://api.github.com/users/fpj/repos","events_url":"https://api.github.com/users/fpj/events{/privacy}","received_events_url":"https://api.github.com/users/fpj/received_events","type":"User","site_admin":false}],"milestone":null,"comments":0,"created_at":"2018-05-02T22:06:29Z","updated_at":"2018-05-02T22:40:14Z","closed_at":null,"author_association":"COLLABORATOR","pull_request":{"url":"https://api.github.com/repos/pravega/pravega/pulls/2539","html_url":"https://github.com/pravega/pravega/pull/2539","diff_url":"https://github.com/pravega/pravega/pull/2539.diff","patch_url":"https://github.com/pravega/pravega/pull/2539.patch"},"body":"**Change log description**\r\n`SegmentAttributeIndex` checks if the main Segment's `isDeleted` flag has been set before making any changes to the Attribute Segment. If so, it automatically deletes the Attribute Segment file. This was originally done as a preemptive operation to make sure this file isn't left orphan.\r\n\r\nIt turns out that, in some scenarios, this is trying to prematurely delete this Segment. When doing so, the `StorageWriter` stubbornly tries to recreate it, however that operation clashes with this check (again) which tries to delete it. Since neither Create nor Delete are atomic operations in `RollingStorage`, it's possible that concurrent calls to these for the same segment will clash at one point and produce weird results. This is exactly what happened in the error reported in the attached issue: a call to `create` interfered with a call to `delete` (this may be the `delete` from the Segment Container or the auto-delete). The likely order of (concurrent) operations was this:\r\n1. `create` was invoked: a new empty header file was made.\r\n2. `delete` was invoked, the header file was sealed.\r\n3. `openWrite` was invoked; a read-only handle was returned since the header was sealed.\r\n4. `delete` finished, thus deleting the header file\r\n5. Subsequent calls to `write` failed due to the handle being read-only (it never made it to the actual Tier2)\r\n\r\nWith this change, we are no longer auto-deleting the Attribute Segment anymore in case we detect the main segment was deleted. This will eliminate the vicious cycle of delete-create-delete-create-... that lead to the `StorageWriter` getting stuck on this.\r\n\r\nThe Attribute Segment will be deleted anyway by the Segment Container when the main segment is deleted. This is well-handled by the `StorageWriter`; when it detects that, it automatically terminates all operations on the affected Segment and skips over the future ones (this is different from the AttributeIndex getting ahead of itself and deleting the Attribute Segment prematurely).\r\n\r\n**Purpose of the change**\r\nFixes #2538.\r\n\r\n**What the code does**\r\nSee Description.\r\n\r\n**How to verify it**\r\nMany runs of E2E integration tests should yield no failure due to this problem.\r\n"},"comment":{"url":"https://api.github.com/repos/pravega/pravega/issues/comments/386143491","html_url":"https://github.com/pravega/pravega/pull/2539#issuecomment-386143491","issue_url":"https://api.github.com/repos/pravega/pravega/issues/2539","id":386143491,"user":{"login":"codecov[bot]","id":22429695,"avatar_url":"https://avatars2.githubusercontent.com/in/254?v=4","gravatar_id":"","url":"https://api.github.com/users/codecov%5Bbot%5D","html_url":"https://github.com/apps/codecov","followers_url":"https://api.github.com/users/codecov%5Bbot%5D/followers","following_url":"https://api.github.com/users/codecov%5Bbot%5D/following{/other_user}","gists_url":"https://api.github.com/users/codecov%5Bbot%5D/gists{/gist_id}","starred_url":"https://api.github.com/users/codecov%5Bbot%5D/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/codecov%5Bbot%5D/subscriptions","organizations_url":"https://api.github.com/users/codecov%5Bbot%5D/orgs","repos_url":"https://api.github.com/users/codecov%5Bbot%5D/repos","events_url":"https://api.github.com/users/codecov%5Bbot%5D/events{/privacy}","received_events_url":"https://api.github.com/users/codecov%5Bbot%5D/received_events","type":"Bot","site_admin":false},"created_at":"2018-05-02T22:40:14Z","updated_at":"2018-05-02T22:40:14Z","author_association":"NONE","body":"# [Codecov](https://codecov.io/gh/pravega/pravega/pull/2539?src=pr&el=h1) Report\n> Merging [#2539](https://codecov.io/gh/pravega/pravega/pull/2539?src=pr&el=desc) into [master](https://codecov.io/gh/pravega/pravega/commit/5f605ce1d7c27abf75b5d22c5567900433e154c9?src=pr&el=desc) will **increase** coverage by `0.04%`.\n> The diff coverage is `100%`.\n\n[![Impacted file tree graph](https://codecov.io/gh/pravega/pravega/pull/2539/graphs/tree.svg?width=650&height=150&src=pr&token=6xOvaR0sIa)](https://codecov.io/gh/pravega/pravega/pull/2539?src=pr&el=tree)\n\n```diff\n@@             Coverage Diff              @@\n##             master    #2539      +/-   ##\n============================================\n+ Coverage     77.62%   77.66%   +0.04%     \n- Complexity     6719     6725       +6     \n============================================\n  Files           523      523              \n  Lines         26713    26711       -2     \n  Branches       2523     2522       -1     \n============================================\n+ Hits          20735    20745      +10     \n+ Misses         4487     4484       -3     \n+ Partials       1491     1482       -9\n```\n\n\n| [Impacted Files](https://codecov.io/gh/pravega/pravega/pull/2539?src=pr&el=tree) | Coverage Δ | Complexity Δ | |\n|---|---|---|---|\n| [...store/server/attributes/SegmentAttributeIndex.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-c2VnbWVudHN0b3JlL3NlcnZlci9zcmMvbWFpbi9qYXZhL2lvL3ByYXZlZ2Evc2VnbWVudHN0b3JlL3NlcnZlci9hdHRyaWJ1dGVzL1NlZ21lbnRBdHRyaWJ1dGVJbmRleC5qYXZh) | `89.52% <100%> (+0.79%)` | `61 <4> (-3)` | :arrow_down: |\n| [...tProcessor/requesthandlers/TruncateStreamTask.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL2lvL3ByYXZlZ2EvY29udHJvbGxlci9zZXJ2ZXIvZXZlbnRQcm9jZXNzb3IvcmVxdWVzdGhhbmRsZXJzL1RydW5jYXRlU3RyZWFtVGFzay5qYXZh) | `92.1% <0%> (-5.27%)` | `15% <0%> (-1%)` | |\n| [.../client/stream/impl/ControllerResolverFactory.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvaW8vcHJhdmVnYS9jbGllbnQvc3RyZWFtL2ltcGwvQ29udHJvbGxlclJlc29sdmVyRmFjdG9yeS5qYXZh) | `82.72% <0%> (-2.73%)` | `7% <0%> (ø)` | |\n| [...o/pravega/segmentstore/server/logs/DurableLog.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-c2VnbWVudHN0b3JlL3NlcnZlci9zcmMvbWFpbi9qYXZhL2lvL3ByYXZlZ2Evc2VnbWVudHN0b3JlL3NlcnZlci9sb2dzL0R1cmFibGVMb2cuamF2YQ==) | `87.86% <0%> (-1.95%)` | `66% <0%> (-2%)` | |\n| [.../main/java/io/pravega/client/stream/StreamCut.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvaW8vcHJhdmVnYS9jbGllbnQvc3RyZWFtL1N0cmVhbUN1dC5qYXZh) | `100% <0%> (ø)` | `2% <0%> (+1%)` | :arrow_up: |\n| [...ntstore/storage/impl/bookkeeper/BookKeeperLog.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-c2VnbWVudHN0b3JlL3N0b3JhZ2UvaW1wbC9zcmMvbWFpbi9qYXZhL2lvL3ByYXZlZ2Evc2VnbWVudHN0b3JlL3N0b3JhZ2UvaW1wbC9ib29ra2VlcGVyL0Jvb2tLZWVwZXJMb2cuamF2YQ==) | `80.68% <0%> (+0.28%)` | `76% <0%> (+1%)` | :arrow_up: |\n| [...pravega/controller/store/stream/ZKStoreHelper.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-Y29udHJvbGxlci9zcmMvbWFpbi9qYXZhL2lvL3ByYXZlZ2EvY29udHJvbGxlci9zdG9yZS9zdHJlYW0vWktTdG9yZUhlbHBlci5qYXZh) | `67.58% <0%> (+1.09%)` | `50% <0%> (ø)` | :arrow_down: |\n| [...a/segmentstore/server/logs/OperationProcessor.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-c2VnbWVudHN0b3JlL3NlcnZlci9zcmMvbWFpbi9qYXZhL2lvL3ByYXZlZ2Evc2VnbWVudHN0b3JlL3NlcnZlci9sb2dzL09wZXJhdGlvblByb2Nlc3Nvci5qYXZh) | `86.24% <0%> (+1.11%)` | `41% <0%> (+3%)` | :arrow_up: |\n| [...egmentstore/server/reading/ContainerReadIndex.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-c2VnbWVudHN0b3JlL3NlcnZlci9zcmMvbWFpbi9qYXZhL2lvL3ByYXZlZ2Evc2VnbWVudHN0b3JlL3NlcnZlci9yZWFkaW5nL0NvbnRhaW5lclJlYWRJbmRleC5qYXZh) | `85.62% <0%> (+1.19%)` | `37% <0%> (+1%)` | :arrow_up: |\n| [...a/io/pravega/common/util/OrderedItemProcessor.java](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvaW8vcHJhdmVnYS9jb21tb24vdXRpbC9PcmRlcmVkSXRlbVByb2Nlc3Nvci5qYXZh) | `88.31% <0%> (+1.29%)` | `21% <0%> (+2%)` | :arrow_up: |\n| ... and [5 more](https://codecov.io/gh/pravega/pravega/pull/2539/diff?src=pr&el=tree-more) | |\n\n------\n\n[Continue to review full report at Codecov](https://codecov.io/gh/pravega/pravega/pull/2539?src=pr&el=continue).\n> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)\n> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`\n> Powered by [Codecov](https://codecov.io/gh/pravega/pravega/pull/2539?src=pr&el=footer). Last update [5f605ce...6ee4bba](https://codecov.io/gh/pravega/pravega/pull/2539?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).\n"}}
{"actor":{"display_login":"codecov"}}