{"action":"created","issue":{"url":"https://api.github.com/repos/tilezen/tilequeue/issues/243","repository_url":"https://api.github.com/repos/tilezen/tilequeue","labels_url":"https://api.github.com/repos/tilezen/tilequeue/issues/243/labels{/name}","comments_url":"https://api.github.com/repos/tilezen/tilequeue/issues/243/comments","events_url":"https://api.github.com/repos/tilezen/tilequeue/issues/243/events","html_url":"https://github.com/tilezen/tilequeue/pull/243","id":258625894,"number":243,"title":"wip - Modify Unit of work on queue","user":{"login":"rmarianski","id":53770,"avatar_url":"https://avatars0.githubusercontent.com/u/53770?v=4","gravatar_id":"","url":"https://api.github.com/users/rmarianski","html_url":"https://github.com/rmarianski","followers_url":"https://api.github.com/users/rmarianski/followers","following_url":"https://api.github.com/users/rmarianski/following{/other_user}","gists_url":"https://api.github.com/users/rmarianski/gists{/gist_id}","starred_url":"https://api.github.com/users/rmarianski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rmarianski/subscriptions","organizations_url":"https://api.github.com/users/rmarianski/orgs","repos_url":"https://api.github.com/users/rmarianski/repos","events_url":"https://api.github.com/users/rmarianski/events{/privacy}","received_events_url":"https://api.github.com/users/rmarianski/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2017-09-18T21:23:51Z","updated_at":"2017-09-19T18:48:07Z","closed_at":null,"author_association":"OWNER","pull_request":{"url":"https://api.github.com/repos/tilezen/tilequeue/pulls/243","html_url":"https://github.com/tilezen/tilequeue/pull/243","diff_url":"https://github.com/tilezen/tilequeue/pull/243.diff","patch_url":"https://github.com/tilezen/tilequeue/pull/243.patch"},"body":"WIP still, but putting it up if you want to take a look to see if I'm taking the right approach.\r\n\r\nI waffled a bit, but ended up settling on modifying the queue implementations to work with raw payloads, and not coordinates. This means that we'll have more concepts and moving parts, but imho is a better design because it means we can mix and match them more readily which will allow us to be more flexible, and it keeps each component simple.\r\nAs part of this, I also teased apart the in flight management, so the only responsibility for queue implementations now is to translate calls to their underlying backing queue.\r\n\r\nThe additional concepts:\r\n\r\n* message marshaller - translates a payload from an individual queue message into multiple coordinates\r\n* message tracker - coordinates between work on the individual tiles and the upstream queue message they correspond to. This will manage the mapping between message handles that tile queues understand, and coordinate handles that the processing pipelines work with.\r\n*  message grouper - takes in a sequence of coordinates, and groups them into cohesive chunks for sending to the queue. In this case, it'll group z10+ tiles into a single chunk based on common z10 parents.\r\n* inflight manager - takes in a sequence of coordinates, and filters out the coordinates that are in the in flight list.\r\n\r\nI've also added a queue \"writer\", which threads all these together. I've verified that this works with test sqs queues, but I haven't updated all the existing code to use this yet. This would be the next step, and it would be a matter of just ensuring that we have configuration to dial this in, and use this everywhere we have the threaded enqueuer currently, eg seeding and intersection code paths.\r\n\r\nThere is a complication that came up thus far, which is that fundamentally the multisqs queue implementation needs to know the zoom level that the payload corresponds to in order to dispatch to the appropriate queue. I handled this by having the enqueue methods take in metadata to specify this at the moment. This is working, but it might be better to instead remove the multisqs implementation and just formalize this bit of logic into its own component and add it to the writer. The mapping logic already gets constructed outside the multisqs implementation (it just gets a lookup object), so this isn't such a big step. Now that I think about it more, I'm leaning towards moving in this direction.\r\n\r\nI also plan on adding more tests throughout, reviewing the existing ones more thoroughly (I only just got them to pass), and documenting these ideas more in the code itself, but I think it's prudent to ask for a review now before spending more time on that.\r\n"},"comment":{"url":"https://api.github.com/repos/tilezen/tilequeue/issues/comments/330634837","html_url":"https://github.com/tilezen/tilequeue/pull/243#issuecomment-330634837","issue_url":"https://api.github.com/repos/tilezen/tilequeue/issues/243","id":330634837,"user":{"login":"nvkelso","id":853051,"avatar_url":"https://avatars0.githubusercontent.com/u/853051?v=4","gravatar_id":"","url":"https://api.github.com/users/nvkelso","html_url":"https://github.com/nvkelso","followers_url":"https://api.github.com/users/nvkelso/followers","following_url":"https://api.github.com/users/nvkelso/following{/other_user}","gists_url":"https://api.github.com/users/nvkelso/gists{/gist_id}","starred_url":"https://api.github.com/users/nvkelso/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nvkelso/subscriptions","organizations_url":"https://api.github.com/users/nvkelso/orgs","repos_url":"https://api.github.com/users/nvkelso/repos","events_url":"https://api.github.com/users/nvkelso/events{/privacy}","received_events_url":"https://api.github.com/users/nvkelso/received_events","type":"User","site_admin":false},"created_at":"2017-09-19T18:48:07Z","updated_at":"2017-09-19T18:48:07Z","author_association":"OWNER","body":"Rob and I chatted IRL about this but I'd like to look at 4 queues:\r\n\r\n- Hotfix path\r\n- zoom 0-9 low-zoom\r\n- zoom 10+ TOI\r\n- zoom 10+ nonTOI"}}
{"actor":{"display_login":"nvkelso"}}