Adar Dembo has posted comments on this change.

Change subject: Non-covering Range Partitions design doc
......................................................................


Patch Set 1:

(12 comments)

I'd like to see more detail on how a client should manage its metadata cache 
when partitions may change. I think it would be useful to break that out into 
its own coherent section.

http://gerrit.cloudera.org:8080/#/c/2772/1/docs/design-docs/non-covering-range-partitions.md
File docs/design-docs/non-covering-range-partitions.md:

Line 10: Currently, Kudu tables create a set of tablets during creation 
according the
Nit: according to


Line 29: necessary for this use-case. Additionaly, dropping range partitions 
enables
Nit: Additionally


Line 69: but schema designers may find it useful to be able to use
       : both options
I see. We should provide good guidance on how to use the two together. As per 
your example below, "Range bounds should be used to specify (potentially 
non-overlapping) ranges. Split rows can be used to further split each range 
into smaller chunks, but beyond this convenience, their general use is 
discouraged."


Line 87:               RANGE BOUND (("North America"), ("North America\0")),
       :               RANGE BOUND (("Europe"), ("Europe\0")),
       :               RANGE BOUND (("Asia"), ("Asia\0"));
This example suggests that it would be useful to express "point" ranges (i.e. 
all of "Europe"), so that schema creators needn't futz with nul terminators and 
repetition in the range bounds expression.


Line 92: ### Inserts and Updates
Nit: just say Writes, since the behavior is the same.


Line 95: scenario in the client, in which the application attempts to write or 
update a
Nit: just write ("update" suggests that you should add "delete" as a case too, 
but write is sufficient).


Line 107: 
Do we need to consider the effect of non-existent range partitions on the 
client's metadata cache? Can it scale to a larger number of tablets?

To be clear, the existence of non-existent range partitions doesn't necessarily 
imply that a table will be split into a larger number of tablets (i.e. that 
could happen with a large number of split rows too), but in practice I think it 
will have that effect.


Line 121: only
        : recontacting the master after a configurable timeout.
Would it be sufficient to recontact the master in the event that a write 
yielded TabletNotPresent? Would that work for both adding or dropping range 
partitions?

Not sure it'll work for scans though, since, as you wrote above, non-existent 
range partitions will be ignored.


Line 134: recognize a dropped tablet when trying to insert or scan the tablet. 
The tablet
Nit: write, not insert


Line 131: Unlike the add range partition case in which a
        : client can not know whether a new range partition has been added 
since the last
        : master lookup, during a drop range partition the client will be able 
to
        : recognize a dropped tablet when trying to insert or scan the tablet
I don't understand this; why aren't the ADD and DROP cases symmetric w.r.t. the 
client? In either case:
1. If the client is writing/scanning on a covered range, the operation will 
succeed.
2. If the client is writing/scanning on a non-covered range, the operation will 
yield TabletNotPresent.


Line 139: non-existent tablet. In the case of an insert or update the 
`TabletNotPresent`
Nit: just write


Line 140: In the case of a scan, no results will be
        : returned from the non-existent tablet.
Should this be configurable? Is there a use case wherein an application _wants_ 
to know that it is scanning a range that is no longer covered by any tablet?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3e530eda60c00faf066c41b6bdb2b37f6d96a5dc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to