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

Change subject: KUDU-3223: Management of per-table level limit
......................................................................


Patch Set 37:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/client/client.h@1809
PS36, Line 1809:  alterations, because the
               :   /// table 'limit' alteration needs the super user per
> nit: I think this would be a bit confusing to users -- "should" is a bit am
Ack


http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/client/client.h@1815
PS36, Line 1815:   /// @param [in] disk_size_limit
> We should also mention that a KuduTableAlterer that is setting limits canno
Ack


http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/client/table_alterer-internal.h
File src/kudu/client/table_alterer-internal.h:

http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/client/table_alterer-internal.h@20
PS36, Line 20: #include <cstdint>
             : #include <map>
             : #include <memo
> nit: please remove the extra line here.
Ack


http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/integration-tests/write_limit-itest.cc
File src/kudu/integration-tests/write_limit-itest.cc:

http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/integration-tests/write_limit-itest.cc@352
PS36, Line 352: ing owner, adding/altering/dropping column
> Can you also add a test that performing any other alter operation while cha
Ack


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

http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/master/catalog_manager.h@274
PS36, Line 274: // It is useful to place size limits on tables that may be 
ingested to
              :     // by long-lived applications. To avoid breaking existing 
tables, there
              :     // is no limit enforced.
> nit: Rather than being specific about Flink, let's keep this a bit more gen
Ack


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

http://gerrit.cloudera.org:8080/#/c/17273/36/src/kudu/master/catalog_manager.cc@3461
PS36, Line 3461: pb.has_table_disk_size_limit()) {
> This will never be true since table_disk_size_limit() and table_row_count_l
Ack



--
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: 37
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: Wed, 05 May 2021 03:17:30 +0000
Gerrit-HasComments: Yes

Reply via email to