joshelser commented on pull request #1935: URL: https://github.com/apache/hbase/pull/1935#issuecomment-656905422
I just had a chat with Surbhi and we walked through the code, end to end, talking about her solution here. tl;dr I'm with her on the solution, barring one exception where we remove the serialized `SpaceQuotaSnapshot` (column `u:p`) for a table which had both a namespace quota (now removed) and a table quota. Removing the `u:p` record when the table quota still applies would result in a RegionServer potentially move a table out of quota violation when it should still remain in violation. A sketch of how space quotas work again, because I had to remind myself: 1. RegionServers report Region space utilization to the master 2. Master holds this in memory, eventually computing table-level and namespace-level utilization. 3. Master computes whether each table is or is not in violation, and persists that (with usage and limit) into a SpaceQuotaSnapshot (`u:p` column) for each table. 4. RegionServers will read these `SpaceQuotaSnapshots` from `hbase:quota` and use this to determine whether or not to reject the new Mutations in `multi()`. That is, when we drop a quota for a namespace, we should clean up all SpaceQuotaSnapshot records in hbase:quota that do not otherwise have a table-level space quota defined on them. for example, ``` hbase(main):015:0> scan 'hbase:quota' ROW COLUMN+CELL n.test column=q:s, timestamp=1591306396362, value=PBUF\x1A\x07\x08.. t.test:table1 column=q:s, timestamp=1591274418036, value=PBUF\x1A\x07\x08.. t.test:table1 column=u:p, timestamp=1591306447760, value=\x0A\x04\x08.. t.test:table2 column=u:p, timestamp=1591306447758, value=\x0A\x04\x08.. ``` when the space quota on namespace `test` is deleted, we would want to delete `t.test:table2 u:p`, but we would not want to delete `t.test:table1 u:p` because there is a quota defined on this table (`t.test:table1 q:s`). Normally, `QuotaObserverChore` (in the master) is pushing new `SpaceQuotaSnapshot` records into `hbase:quota`, but this class doesn't handle the cleanup. The method that Surbhi has modified is what MasterQuotaManager is ultimately calling (after a couple of levels of indirection), so making this change inside `QuotaUtil` appears to be the right thing to me, hiding the complexity of what it means to delete a namespace quota :) ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
