Skip to content

Sharding Pattern #1056

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Nov 8, 2019
Merged

Sharding Pattern #1056

merged 5 commits into from
Nov 8, 2019

Conversation

Azureyjt
Copy link
Contributor

@Azureyjt Azureyjt commented Nov 7, 2019

This PR should fix issue #454.

Three types of sharding strategy are implemented:

  • Range
  • Lookup
  • Hash

@Azureyjt
Copy link
Contributor Author

Azureyjt commented Nov 7, 2019

@iluwatar Please kindly review this PR.

iluwatar
iluwatar requested changes Nov 7, 2019
View reviewed changes
sharding/README.md Outdated

## Credits

* [Cloud Design Patterns: Prescriptive Architecture Guidance for Cloud Applications - Sharding Pattern]([https://docs.microsoft.com/en-us/previous-versions/msp-n-p/dn589797(v=pandp.10)?redirectedfrom=MSDN](https://docs.microsoft.com/en-us/previous-versions/msp-n-p/dn589797(v=pandp.10)?redirectedfrom=MSDN)) No newline at end of file
Copy link
Owner

Choose a reason for hiding this comment

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

The link seems to there twice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 13ae8d5

sharding/src/main/java/com/iluwatar/sharding/App.java Outdated
shard2.clearData();
shard3.clearData();

return;
Copy link
Owner

Choose a reason for hiding this comment

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

Return is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 13ae8d5

@iluwatar iluwatar self-assigned this Nov 7, 2019
iluwatar
iluwatar requested changes Nov 7, 2019
View reviewed changes
Copy link
Owner

@iluwatar iluwatar left a comment

Choose a reason for hiding this comment

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

Checkstyle with Google Checks reports many issues - should be resolved before merge

@Azureyjt
Copy link
Contributor Author

Azureyjt commented Nov 7, 2019

Checkstyle with Google Checks reports many issues - should be resolved before merge

Hi iluwatar,

Thank you for reviewing the code. I'm sorry that I forgot to check the checkstyle report, and now all the code style issues are fixed. Please kindly check it.

@iluwatar iluwatar merged commit 1fa8a60 into iluwatar:master Nov 8, 2019
@iluwatar
Copy link
Owner

iluwatar commented Nov 8, 2019

@Azureyjt no problem, it's easy to miss now since Checkstyle doesn't fail the build. Once we get #1021 done, we can make it strict.
Thanks for the contribution once again, you're becoming regular here ☺️

@iluwatar iluwatar added this to the 1.22.0 milestone Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iluwatar iluwatar iluwatar approved these changes

Assignees

@iluwatar iluwatar

Labels

status: under review

Projects

None yet

Milestone

1.22.0

Development

Successfully merging this pull request may close these issues.

2 participants

@Azureyjt @iluwatar