{"action":"created","issue":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3500","repository_url":"https://api.github.com/repos/zfsonlinux/zfs","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3500/labels{/name}","comments_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3500/comments","events_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3500/events","html_url":"https://github.com/zfsonlinux/zfs/issues/3500","id":89071732,"number":3500,"title":"dnode accounting for quotas","user":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/zfsonlinux/zfs/labels/Feature","name":"Feature","color":"02d7e1"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/zfsonlinux/zfs/milestones/11","html_url":"https://github.com/zfsonlinux/zfs/milestones/0.7.0","labels_url":"https://api.github.com/repos/zfsonlinux/zfs/milestones/11/labels","id":7013,"number":11,"title":"0.7.0","description":"ZFS on Linux v0.7.0 - Memory management\r\n\r\n","creator":{"login":"behlendorf","id":148917,"avatar_url":"https://avatars.githubusercontent.com/u/148917?v=3","gravatar_id":"","url":"https://api.github.com/users/behlendorf","html_url":"https://github.com/behlendorf","followers_url":"https://api.github.com/users/behlendorf/followers","following_url":"https://api.github.com/users/behlendorf/following{/other_user}","gists_url":"https://api.github.com/users/behlendorf/gists{/gist_id}","starred_url":"https://api.github.com/users/behlendorf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/behlendorf/subscriptions","organizations_url":"https://api.github.com/users/behlendorf/orgs","repos_url":"https://api.github.com/users/behlendorf/repos","events_url":"https://api.github.com/users/behlendorf/events{/privacy}","received_events_url":"https://api.github.com/users/behlendorf/received_events","type":"User","site_admin":false},"open_issues":52,"closed_issues":284,"state":"open","created_at":"2011-05-03T17:38:46Z","updated_at":"2016-09-02T05:55:49Z","due_on":null,"closed_at":null},"comments":32,"created_at":"2015-06-17T17:03:06Z","updated_at":"2016-09-02T18:21:08Z","closed_at":null,"body":"During the OpenZFS summit @thegreatgazoo  and I had a chance to discuss this. We came up with a nice clean solution based on @johannlombardi initial patch. If Issac has the time to do the heavy lifting on this I'm happy to work with him to get the patch reviewed and in to a form where it can be merged. There's a fair bit of work remaining to get it where it needs to be. But it should be pretty straight forward:\r\n\r\nRequired functionality:\r\n* Add a feature flag.\r\n* Rework the code so this feature can be enabled without rewiring every dnode.\r\n* Rework the code to use sync tasks so it can be resumed across pool import/exports.\r\n* Update the zpool status command to provide status information while the feature is being enabled.\r\n* Add the ioctl() handlers so utilities such as repquota work.\r\n* Update send/recv to handle this change.\r\n* Update the zfs(8) and zpool-features(5) man pages.\r\n\r\nImplementation details:\r\n\r\n> One way to enable it without rewriting every dnode is to take a snapshot\r\n\r\nThis is a good idea but we can do better. If we were to use full snapshots there are some significant downsides.\r\n\r\n* We'd need to take a snapshot per dataset and it's not uncommon for pools to have 1000s, 10,000s, or 100,000s of datasets. Doubling this just to enable the feature is a bit heavy handed.\r\n* While the snapshots exist we can't free any data in the pool since it will be referenced by a snapshot. This would be problematic for pools which are already near capacity.\r\n* I suspect cleanly handling all the possible failure modes will be fairly complicated. You'll need to do the creation of all the snapshots in a sync task and be able to unwind them all in the event of a failure. You'll also want to do it in a single tx so that either all the snapshots exist or none of them do. When we're talking about a large number of snapshots this may take a significant amount of time (several seconds).\r\n* Doing any operation which spans datasets complicates things considerably. If this could be avoided it would greatly simplify the problem.\r\n\r\nLuckily, for this specific case a full snapshot isn't needed. Storing the TXG number in which the feature was enabled and the per-dataset dnode number for the traversal is enough. This is possible because every dnode already stores the TXG number it was originally allocated in (dn->dn_allocated_txg). We can also leverage the fact that the traversal will strictly happen for lowest to highest numbered dnodes. Which means we can split the problem up like this:\r\n\r\n* Newly allocated dnodes always update the quota ZAP\r\n* Freed dnodes update the quota ZAP as follows\r\n * if (dn->dn_object < dataset->scan_object): dnode has been traversed by scan update quota ZAP\r\n * if (dn->dn_object >= dataset->scan_object): dnode has NOT been traversed by scan no update needed\r\n* Dnode traversal scan, this can be done with the existing dnode iterator\r\n * if (dn->dn_allocated_txg < feature_txg): dnode is not accounted for update quota ZAP\r\n * if (dn->dn_allocated_txg >= feature_txg): dnode is new has been accounted for during create no update needed\r\n\r\nThe traversal part of this would need to be done in small batches in a sync task. This would allow us to transactional update the dataset->scan_object on disk so the traversal can be resumed and it simplifies concurrency concerns. Doing it this way addresses my concerns above and nicely simplifies the logic and the amount of code needed. For example, all that's needed to abort the entire operation or disable the feature is to stop the traversal sync task (if running) and remove the two new ZAPs."},"comment":{"url":"https://api.github.com/repos/zfsonlinux/zfs/issues/comments/244451110","html_url":"https://github.com/zfsonlinux/zfs/issues/3500#issuecomment-244451110","issue_url":"https://api.github.com/repos/zfsonlinux/zfs/issues/3500","id":244451110,"user":{"login":"jxiong","id":472018,"avatar_url":"https://avatars.githubusercontent.com/u/472018?v=3","gravatar_id":"","url":"https://api.github.com/users/jxiong","html_url":"https://github.com/jxiong","followers_url":"https://api.github.com/users/jxiong/followers","following_url":"https://api.github.com/users/jxiong/following{/other_user}","gists_url":"https://api.github.com/users/jxiong/gists{/gist_id}","starred_url":"https://api.github.com/users/jxiong/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jxiong/subscriptions","organizations_url":"https://api.github.com/users/jxiong/orgs","repos_url":"https://api.github.com/users/jxiong/repos","events_url":"https://api.github.com/users/jxiong/events{/privacy}","received_events_url":"https://api.github.com/users/jxiong/received_events","type":"User","site_admin":false},"created_at":"2016-09-02T18:21:08Z","updated_at":"2016-09-02T18:21:08Z","body":">  Functionally, these things are equivalent. If there's a problem with upgrading mounted datasets it's an issue for both approaches.\r\n\r\nI thought you suggested to upgrade all active dataset in the system - this will pose problems of race condition, for example, during this process a new dataset is mounted, etc. This may need some efforts to make it correct. I meant if this were useful code and exercised often, it would be worth doing.\r\n\r\n> In fact, a dmu_objset_userused_enabled() check already happens in spa_sync()->dsl_pool_sync()->dsl_dataset_sync()->dmu_objset_sync() for sightly different reasons. This may be a better place to trigger the upgrade.\r\n\r\nThe problem is that we have to dirty all dnodes in the dataset, which is not feasible to perform in sync context because this will hold txg sync way too long if the dataset has huge amount of objects.\r\n\r\nI dislike the current way of upgrading either - actually I often forgot the command to upgrade individual dataset. But this is the best way I can find so far."}}
{"actor":{"display_login":"jxiong"}}