Skip to content

GEODE-3063 partition resolver doc revisions#1105

Merged
karensmolermiller merged 2 commits into
apache:developfrom
karensmolermiller:feature/GEODE-3063-2
Dec 11, 2017
Merged

GEODE-3063 partition resolver doc revisions#1105
karensmolermiller merged 2 commits into
apache:developfrom
karensmolermiller:feature/GEODE-3063-2

Conversation

@karensmolermiller

Copy link
Copy Markdown
Contributor

My previous PR for this work has been closed. This PR now contains a single commit for the entire change set. The sections on partition resolvers have been completely rewritten and reorganized. All review comments in the previous PR and in the Geode JIRA (GEODE-3063) have been addressed.

Please review/re-review this new PR.

@davebarnes97 davebarnes97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! I have one question about verbiage you inherited. Each of the three sections describing a type of partitioning (fixed, custom, standard) begins with "By default <product> partitions each data entry into a bucket..." Is "partitions into" the right verb here? I'm thinking partitioning is for regions, so for data items "maps into" or "assigns to" might be better. Your call.

@karensmolermiller

Copy link
Copy Markdown
Contributor Author

Here are Darrel's comments on this PR:
I would change: assigns new entries to buckets based on the entry key contents
to: assigns new entries to buckets based on the entry key's hash code

I think it is confusing to abbreviate : "standard custom partitioning" by dropping "custom" from it. So I would say we should include "custom" in both "standard partitioning" and "fixed partitioning".

change: "Using a custom class."
to: "Using the partition resolver region attribute."
Throughout this do not use the term "custom class" but instead use "partition resolver region attribute".
Even though this is, strictly speaking, a "partition attribute", I think it is better to call it a "region attribute" since the attributes name is "partition resolver".

change: "Specify this class as the partition resolver during region creation."
to: "Implement the PartitionResolver interface with your own class and then specify that class as the partition resolver during region creation."

change: "For keys implemented as objects, define the interface within the key’s class."
to: "Have the entry key's class implement the PartitionResolver interface."

change: "Using the cache callback class."
to: "Using the callback argument's class."

change: "Implement the interface within a cache callback’s class."
to: "Have the class that implements your callback argument implement the PartitionResolver interface."
Call it "callback argument" in this section not just "callback" or "cache callback".

change: "Implementing the String-Based Partition Resolver"
to "Using the String Prefix Partition Resolver"
In general replace "String-Based" with "String Prefix".
Add to that section "This class can be used as the partition resolver region attribute." and/or refer them to the section on configuring it.

In the section titled "Configuring Standard Partitioning" change the title to "Configuring the Partition Resolver Region Attribute"
and change the section to focus on this instead of "custom partitioning". For example I think you can just drop this sentence:
"How you do this depends on where you chose to implement your custom partitioning."
What it depends on is how you are configuring the product. You should start with gfsh, then java, and then xml. This ordering should be used throughout all the examples. We want to push gfsh.

Delete these items from the configuration section:
Entry key. Use the key object with the resolver implementation for every entry operation.
Cache callback argument. Provide the argument to every call that accesses an entry. This restricts you to calls that take a callback argument.

Delete this line: Define the class for the region at creation. The resolver will be used for every entry operation. Use one of these methods:

@karensmolermiller karensmolermiller merged commit 08944b0 into apache:develop Dec 11, 2017
PivotalSarge pushed a commit to upthewaterspout/geode that referenced this pull request Dec 14, 2017
* GEODE-3063 partition resolver doc revisions

* GEODE-3063: Revisions on docs of custom partitioning
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants