Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17273 )

Change subject: KUDU-3223: Quota management for table write
......................................................................


Patch Set 28:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17273/26/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/17273/26/src/kudu/master/catalog_manager.cc@2894
PS26, Line 2894:   // Modify the table limit by admin. Considering this 
operation
               :   // is done by admin, it should not combine with other table 
alterations.
               :   bool table_limit_change = req.has_disk_size_limit() ||
               :                             req.has_row_count_limit();
               :   bool other_schema_change = req.has_new_table_name() ||
               :  
> Yes, this can be improved. But I don't think we can save any space. Conside
I mean the SysTabletsEntryPB will be smaller, and that protobuf message is 
written to disk. I'm fine letting the request use a special value for clearing 
the limits, but I don't see a reason we need to store a value for the limit 
when there is none.


http://gerrit.cloudera.org:8080/#/c/17273/28/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/17273/28/src/kudu/master/catalog_manager.cc@2904
PS28, Line 2904:     if (other_schema_change) {
               :       return SetupError(Status::ConfigurationError(
               :                         "Alter table limit cannot be combined 
with other alterations"),
               :                         resp, MasterErrorPB::UNKNOWN_ERROR);
Let's move this to step 1 so we catch bad requests early.



--
To view, visit http://gerrit.cloudera.org:8080/17273
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dbf365ad59f17c0a4e2e7ea6a5afaa7680724b0
Gerrit-Change-Number: 17273
Gerrit-PatchSet: 28
Gerrit-Owner: Hongjiang Zhang <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Hongjiang Zhang <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 29 Apr 2021 06:05:17 +0000
Gerrit-HasComments: Yes

Reply via email to