{"action":"created","issue":{"url":"https://api.github.com/repos/prestodb/presto/issues/11253","repository_url":"https://api.github.com/repos/prestodb/presto","labels_url":"https://api.github.com/repos/prestodb/presto/issues/11253/labels{/name}","comments_url":"https://api.github.com/repos/prestodb/presto/issues/11253/comments","events_url":"https://api.github.com/repos/prestodb/presto/issues/11253/events","html_url":"https://github.com/prestodb/presto/issues/11253","id":349886159,"node_id":"MDU6SXNzdWUzNDk4ODYxNTk=","number":11253,"title":"Split scheduling is broken for colocated join","user":{"login":"haozhun","id":2020503,"node_id":"MDQ6VXNlcjIwMjA1MDM=","avatar_url":"https://avatars3.githubusercontent.com/u/2020503?v=4","gravatar_id":"","url":"https://api.github.com/users/haozhun","html_url":"https://github.com/haozhun","followers_url":"https://api.github.com/users/haozhun/followers","following_url":"https://api.github.com/users/haozhun/following{/other_user}","gists_url":"https://api.github.com/users/haozhun/gists{/gist_id}","starred_url":"https://api.github.com/users/haozhun/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/haozhun/subscriptions","organizations_url":"https://api.github.com/users/haozhun/orgs","repos_url":"https://api.github.com/users/haozhun/repos","events_url":"https://api.github.com/users/haozhun/events{/privacy}","received_events_url":"https://api.github.com/users/haozhun/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-08-13T04:44:53Z","updated_at":"2018-08-13T04:49:03Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"There are two bugs that affect scheduling for fragment/stage with more than one scan nodes. Only colocated join can create such fragment/stage at this time.\r\n\r\nThe combined outcome is that there is no back pressure for split scheduling for all but the first scan node (in source scheduling order) in a stage. However, if bug 2 gets fixed, it would lead to scheduling deadlocks if bug 1 is not also fixed. Bug 1 have limited user visibility because it's more or less mitigated by bug 2.\r\n\r\n### bug 1\r\n\r\nWhen a fragment/stage contains multiple scan nodes. None of the scan nodes will receive a `TaskSource` where `TaskSource.noMoreSplits = true` until all workers receive all the splits for all the scan nodes in the stage. \r\n\r\nIn `SqlStageExecution`, there are 2 places where `completeSources` variable is updated. One in `schedulingComplete`, which happens after the entire stage finishes scheduling. The other in `addExchangeLocations`, which isn't relevant for scan nodes.\r\n\r\nIn `SqlStageExecution`, there are 3 places where `task.noMoreSplits` is invoked. It's invoked in `scheduleTask`, where `task.noMoreSplits` is invoked for each element in `completeSources`. The other two are `schedulingComplete` and `addExchangeLocation`.\r\n\r\nThis bug seems unlikely because the bug should have caused frequent query deadlock. That leads me to bug 2.\r\n\r\n### bug 2\r\n\r\nIn `PipelineContext.getPipelineStatus`, `queuedDrivers` is computed by looking at `DriverContext`s. However, note that in `SqlTaskExecution.schedulePartitionedSource`, there is this concept of `pendingSplitsByPlanNode`. `pendingSplitsByPlanNode` buffers splits for scan nodes that aren't yet eligible to schedule because another scan node who is ahead in term of source scheduling order hasn't finished scheduling.\r\n\r\nSpecifically, that \"another scan node\" is the first scan node (in source scheduling order). Due to bug 1, it will not finish scheduling until all splits for the stage are delivered to workers.\r\n\r\n`queuedDrivers` should include splits in `pendingSplitsByPlanNode` although a `DriverSplitRunner` is yet to be created for those splits. The fact that the worker chose to defer the creation of those drivers is an implementation detail. Conceptually, those splits have been delivered to the worker, and the worker has created the drivers for them although those drivers are \"blocked\" (not runnable).\r\n\r\nNow look at `NodeScheduler.selectDistributionNodes`. It depends on `NodeAssignmentStats.getTotalSplitCount`. It is effectively `queuedDrivers` + recent assignment. While recent assignment would increment, it is reset to zero whenever the split is delivered to workers. `queuedDrivers` would be a small number (and eventually hit zero) due to this bug.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/prestodb/presto/issues/comments/412407620","html_url":"https://github.com/prestodb/presto/issues/11253#issuecomment-412407620","issue_url":"https://api.github.com/repos/prestodb/presto/issues/11253","id":412407620,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMjQwNzYyMA==","user":{"login":"haozhun","id":2020503,"node_id":"MDQ6VXNlcjIwMjA1MDM=","avatar_url":"https://avatars3.githubusercontent.com/u/2020503?v=4","gravatar_id":"","url":"https://api.github.com/users/haozhun","html_url":"https://github.com/haozhun","followers_url":"https://api.github.com/users/haozhun/followers","following_url":"https://api.github.com/users/haozhun/following{/other_user}","gists_url":"https://api.github.com/users/haozhun/gists{/gist_id}","starred_url":"https://api.github.com/users/haozhun/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/haozhun/subscriptions","organizations_url":"https://api.github.com/users/haozhun/orgs","repos_url":"https://api.github.com/users/haozhun/repos","events_url":"https://api.github.com/users/haozhun/events{/privacy}","received_events_url":"https://api.github.com/users/haozhun/received_events","type":"User","site_admin":false},"created_at":"2018-08-13T04:49:03Z","updated_at":"2018-08-13T04:49:03Z","author_association":"CONTRIBUTOR","body":"cc @dain "}}
{"actor":{"display_login":"haozhun"}}