{"action":"created","issue":{"url":"https://api.github.com/repos/hathitrust/datasets/issues/37","repository_url":"https://api.github.com/repos/hathitrust/datasets","labels_url":"https://api.github.com/repos/hathitrust/datasets/issues/37/labels{/name}","comments_url":"https://api.github.com/repos/hathitrust/datasets/issues/37/comments","events_url":"https://api.github.com/repos/hathitrust/datasets/issues/37/events","html_url":"https://github.com/hathitrust/datasets/issues/37","id":219021734,"number":37,"title":"only enqueue CreateJob if volume on disk has changed","user":{"login":"aelkiss","id":3856018,"avatar_url":"https://avatars1.githubusercontent.com/u/3856018?v=3","gravatar_id":"","url":"https://api.github.com/users/aelkiss","html_url":"https://github.com/aelkiss","followers_url":"https://api.github.com/users/aelkiss/followers","following_url":"https://api.github.com/users/aelkiss/following{/other_user}","gists_url":"https://api.github.com/users/aelkiss/gists{/gist_id}","starred_url":"https://api.github.com/users/aelkiss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aelkiss/subscriptions","organizations_url":"https://api.github.com/users/aelkiss/orgs","repos_url":"https://api.github.com/users/aelkiss/repos","events_url":"https://api.github.com/users/aelkiss/events{/privacy}","received_events_url":"https://api.github.com/users/aelkiss/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/hathitrust/datasets/milestones/1","html_url":"https://github.com/hathitrust/datasets/milestone/1","labels_url":"https://api.github.com/repos/hathitrust/datasets/milestones/1/labels","id":2289418,"number":1,"title":"stable","description":"Complete dataset with stable process for updates as promised.","creator":{"login":"aelkiss","id":3856018,"avatar_url":"https://avatars1.githubusercontent.com/u/3856018?v=3","gravatar_id":"","url":"https://api.github.com/users/aelkiss","html_url":"https://github.com/aelkiss","followers_url":"https://api.github.com/users/aelkiss/followers","following_url":"https://api.github.com/users/aelkiss/following{/other_user}","gists_url":"https://api.github.com/users/aelkiss/gists{/gist_id}","starred_url":"https://api.github.com/users/aelkiss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aelkiss/subscriptions","organizations_url":"https://api.github.com/users/aelkiss/orgs","repos_url":"https://api.github.com/users/aelkiss/repos","events_url":"https://api.github.com/users/aelkiss/events{/privacy}","received_events_url":"https://api.github.com/users/aelkiss/received_events","type":"User","site_admin":false},"open_issues":11,"closed_issues":10,"state":"open","created_at":"2017-01-31T13:36:03Z","updated_at":"2017-04-03T18:37:29Z","due_on":null,"closed_at":null},"comments":1,"created_at":"2017-04-03T18:30:32Z","updated_at":"2017-04-04T11:57:39Z","closed_at":null,"body":"right now `VolumeCreator#should_write_zip?` checks whether the source path is newer than the destination zip. That means that if we run a full refresh on the data set, it will queue a CreateJob for every volume in the set, which is a lot of extra work given that jobs are relatively expensive and checking the file time is relatively inexpensive. It also makes logging for incremental rsync (#10) more difficult if the plan is to log all jobs - since not all jobs actually result in a changed zip on disk. I know we were OK with the incremental being a superset of the actual changed volumes, but we should produce an incremental list that's actually useful.\r\n\r\nSome alternatives to only enqueueing a job if the volume on disk has changed:\r\n- provide a list of volumes to update instead of running against the entire dataset. That's potentially error prone since it would mean we need an independent (and likely untested) method of generating the set of volumes to update. \r\n- we could generate a list of changed volumes by having `VolumeCreator` do some logging itself (probably the easiest to reason about, not sure how easy actually having VolumeCreator do logging would be though)\r\n- or by running a `find` on the filesystem after all jobs have completed (maybe a reasonable compromise?)"},"comment":{"url":"https://api.github.com/repos/hathitrust/datasets/issues/comments/291477995","html_url":"https://github.com/hathitrust/datasets/issues/37#issuecomment-291477995","issue_url":"https://api.github.com/repos/hathitrust/datasets/issues/37","id":291477995,"user":{"login":"aelkiss","id":3856018,"avatar_url":"https://avatars1.githubusercontent.com/u/3856018?v=3","gravatar_id":"","url":"https://api.github.com/users/aelkiss","html_url":"https://github.com/aelkiss","followers_url":"https://api.github.com/users/aelkiss/followers","following_url":"https://api.github.com/users/aelkiss/following{/other_user}","gists_url":"https://api.github.com/users/aelkiss/gists{/gist_id}","starred_url":"https://api.github.com/users/aelkiss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/aelkiss/subscriptions","organizations_url":"https://api.github.com/users/aelkiss/orgs","repos_url":"https://api.github.com/users/aelkiss/repos","events_url":"https://api.github.com/users/aelkiss/events{/privacy}","received_events_url":"https://api.github.com/users/aelkiss/received_events","type":"User","site_admin":false},"created_at":"2017-04-04T11:57:39Z","updated_at":"2017-04-04T11:57:39Z","body":"Performance: Let's try it and see. The option to disable forking for workers entirely for workers is straightforward and supported, so if performance is poor that's an easy first place to start.\r\n\r\nIncremental diffs: I think they plan on running a full sync in June but want to do an incremental sync before that. Worst case we can plan on producing a list of changed files with find. I still think it would be useful to have the individual workers log, though.\r\n\r\nEnqueuing jobs based on the state of the filesystem: I think it will only really matter for this first update, and I agree it isn't worth changing design decisions for that."}}
{"actor":{"display_login":"aelkiss"}}