You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2020/04/26 15:30:23 UTC
[GitHub] [shardingsphere] strongduanmu opened a new pull request #5327: create RANGE sharding algorithm
strongduanmu opened a new pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327
Fixes #5280 .
Changes proposed in this pull request:
- create RANGE sharding algorithm.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619721301
> @strongduanmu
>
> Hi, I am glad to read your organized and straightforward code. :)
> There is my concern about `range.partition.split.value` which is the key point of this algorithm.
>
> Users need to list all endpoints of each range partitions, i.e, 10,20,30,40...
> Besides, it generates complicated code handling. Do you think it is better to prefer the partition volume than listing all endpoints? That is, in this case: instead of `range.partition.split.value=10,20,30,40`, maybe we can consider `partition.volume = 10`. I am not sure, so I'd like to listen to your voice. 😀
@tristaZero Partition volume looks more concise, but it can only split all values into same interval partition except the first and last partition, this way may not meet the scene of custom partition range.
Users use partition volume to split all values, and may encounter some scenarios where the amount of partition data is small. At this time, it may be more desirable to be able to merge multiple partition ranges, that is, custom partition ranges.
Is it better to provide two Range algorithms? One meets the requirements of flexible scenarios, and the other is more suitable for general scenarios.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619722582
> BTW, instead of `Fixes #5280 .` which makes this issue closed after merging, `Ref #5280` is suggested, as we still welcome any other great ideas. :)
Okay, I will modify it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620431368
> After more consideration, the second one `init()` is not necessary, maybe.
> At first, I considered to initialize `partitionRangeMap` in init(). However, that means `partitionRangeMap` will not be a final member. If so, at the multi-thread condition, it is not safe, which is more important than the performance of `doSharding()`.
> What's your idea about this one? Wait for your kind reply.
>
> Trista
Hi, @tristaZero , I agree with you that thread safety is more important. We can use another way to improve performance by providing a `useCache` parameter.
If users configured fewer partitions, the performance should be able to meet the requirements. But if the num of partition ranges is very large, such as 100 partitions, users can set `useCache` parameter to `true`, use the cache to improve performance, just like SQLParseResultCache.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620402388
How about `range.partitions`?
@strongduanmu
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619679212
@strongduanmu
Hi, I am glad to read your organized and straightforward code. :)
There is my concern about `range.partition.split.value` which is the key point of this algorithm.
Users need to list all endpoints of each range partitions, i.e, 10,20,30,40...
Besides, it generates complicated code handling. Do you think it is better to prefer the partition volume than listing all endpoints? That is, in this case: instead of `range.partition.split.value=10,20,30,40`, maybe we can consider `partition.volume = 10`. I am not sure, so I'd like to listen to your voice. 😀
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415567642
##########
File path: sharding-core/sharding-core-common/src/test/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/range/RangeShardingAlgorithmTest.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding.range;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Range;
+import org.apache.shardingsphere.api.config.sharding.strategy.StandardShardingStrategyConfiguration;
+import org.apache.shardingsphere.core.strategy.algorithm.sharding.RangeShardingAlgorithm;
+import org.apache.shardingsphere.core.strategy.route.standard.StandardShardingStrategy;
+import org.apache.shardingsphere.core.strategy.route.value.ListRouteValue;
+import org.apache.shardingsphere.core.strategy.route.value.RangeRouteValue;
+import org.apache.shardingsphere.core.strategy.route.value.RouteValue;
+import org.apache.shardingsphere.underlying.common.config.properties.ConfigurationProperties;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+public final class RangeShardingAlgorithmTest {
+
+ private StandardShardingStrategy shardingStrategy;
+
+ @Before
+ public void setUp() {
+ RangeShardingAlgorithm shardingAlgorithm = new RangeShardingAlgorithm();
+ shardingAlgorithm.getProperties().setProperty("range.partition.split.value", "1,5,10");
+ StandardShardingStrategyConfiguration shardingStrategyConfig = new StandardShardingStrategyConfiguration("order_id", shardingAlgorithm);
+ shardingStrategy = new StandardShardingStrategy(shardingStrategyConfig);
+ }
+
+ @Test
+ public void assertPreciseDoSharding() {
+ List<String> availableTargetNames = Lists.newArrayList("t_order_0", "t_order_1", "t_order_2", "t_order_3");
+ List<RouteValue> shardingValues = Lists.newArrayList(new ListRouteValue<>("order_id", "t_order", Lists.newArrayList(0L, 1L, 2L, 4L, 7L)));
+ Collection<String> actual = shardingStrategy.doSharding(availableTargetNames, shardingValues, new ConfigurationProperties(new Properties()));
+ assertThat(actual.size(), is(3));
Review comment:
I got it, thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415547163
##########
File path: sharding-core/sharding-core-common/src/test/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/range/RangeShardingAlgorithmTest.java
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding.range;
+
+import com.google.common.collect.Lists;
+import com.google.common.collect.Range;
+import org.apache.shardingsphere.api.config.sharding.strategy.StandardShardingStrategyConfiguration;
+import org.apache.shardingsphere.core.strategy.algorithm.sharding.RangeShardingAlgorithm;
+import org.apache.shardingsphere.core.strategy.route.standard.StandardShardingStrategy;
+import org.apache.shardingsphere.core.strategy.route.value.ListRouteValue;
+import org.apache.shardingsphere.core.strategy.route.value.RangeRouteValue;
+import org.apache.shardingsphere.core.strategy.route.value.RouteValue;
+import org.apache.shardingsphere.underlying.common.config.properties.ConfigurationProperties;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
+public final class RangeShardingAlgorithmTest {
+
+ private StandardShardingStrategy shardingStrategy;
+
+ @Before
+ public void setUp() {
+ RangeShardingAlgorithm shardingAlgorithm = new RangeShardingAlgorithm();
+ shardingAlgorithm.getProperties().setProperty("range.partition.split.value", "1,5,10");
+ StandardShardingStrategyConfiguration shardingStrategyConfig = new StandardShardingStrategyConfiguration("order_id", shardingAlgorithm);
+ shardingStrategy = new StandardShardingStrategy(shardingStrategyConfig);
+ }
+
+ @Test
+ public void assertPreciseDoSharding() {
+ List<String> availableTargetNames = Lists.newArrayList("t_order_0", "t_order_1", "t_order_2", "t_order_3");
+ List<RouteValue> shardingValues = Lists.newArrayList(new ListRouteValue<>("order_id", "t_order", Lists.newArrayList(0L, 1L, 2L, 4L, 7L)));
+ Collection<String> actual = shardingStrategy.doSharding(availableTargetNames, shardingValues, new ConfigurationProperties(new Properties()));
+ assertThat(actual.size(), is(3));
Review comment:
It is suggested to assert some of the specific targets, not only `size()`.
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ Collection<String> result = new LinkedHashSet<>(availableTargetNames.size());
+ for (long value = shardingValue.getValueRange().lowerEndpoint(); value <= shardingValue.getValueRange().upperEndpoint(); value++) {
Review comment:
It is likely that`value++` is time-consuming if there is a vast range.
Here are some of my ideas.
For instance,the shardings are as follows,
1,2 | 3,4,5 | 6,7,8,9 | 10,11
So the `range.partition.split.value` is `2,5,9,11`
Imaging the query condition is `2 =< x <= 9`, then we just need to calcute 2==2 and 9<11, therefore the results is `0,1,2`.
That is, we can get the critical values from `range.partition.split.value`, so we only need to compare the bottom point and the top point with those critical values.
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ Collection<String> result = new LinkedHashSet<>(availableTargetNames.size());
+ for (long value = shardingValue.getValueRange().lowerEndpoint(); value <= shardingValue.getValueRange().upperEndpoint(); value++) {
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, value))) {
+ result.add(each);
+ }
+ }
+ }
+ return result;
+ }
+
+ private Map<Integer, Range<Long>> getPartitionRangeMap() {
+ List<Long> splitValues = Splitter.on(",").trimResults().splitToList(properties.get(RANGE_PARTITION_SPLIT_VALUE).toString())
+ .stream().map(Longs::tryParse).filter(Objects::nonNull).sorted().collect(Collectors.toList());
+ Preconditions.checkArgument(CollectionUtils.isNotEmpty(splitValues), "Range sharding algorithm range partition split value is not valid.");
+ Map<Integer, Range<Long>> partitionRangeMap = Maps.newHashMapWithExpectedSize(splitValues.size() + 1);
+ for (int i = 0; i < splitValues.size(); i++) {
+ Long splitValue = splitValues.get(i);
+ if (i == 0) {
+ partitionRangeMap.put(i, Range.lessThan(splitValue));
+ } else {
+ Long previousSplitValue = splitValues.get(i - 1);
+ partitionRangeMap.put(i, Range.closedOpen(previousSplitValue, splitValue));
+ }
+ if (i == splitValues.size() - 1) {
+ partitionRangeMap.put(i + 1, Range.atLeast(splitValue));
+ }
+ }
+ return partitionRangeMap;
+ }
+
+ private String getPartition(final Map<Integer, Range<Long>> partitionRangeMap, final Long value) {
+ for (Map.Entry<Integer, Range<Long>> entry : partitionRangeMap.entrySet()) {
+ if (entry.getValue().contains(value)) {
+ return entry.getKey().toString();
+ }
+ }
+ return partitionRangeMap.keySet().stream().mapToInt(Integer::valueOf).max().toString();
+ }
+
+ @Override
+ public String getType() {
+ return "RANGE";
+ }
+
+ @Override
Review comment:
@Setter and @Getter are recommended.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620370632
After more consideration, the second one `init()` is not necessary, maybe.
At first, I considered to initialize `partitionRangeMap` in init(). However, that means `partitionRangeMap` will not be a final member. If so, at the multi-thread condition, it is not safe, which is more important than the performance of `doSharding()`.
What's your idea about this one? Wait for your kind reply.
Trista
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620350166
Hi @strongduanmu It seems this PR is excellent, so I can not wait to merge it, thanks for your effort! However, there are still some questions that remained for us,
1. Are there any better names for range.partition.split.value to make users easily get its meaning. Like range.partition.upper.points or range.partition.boundary.points?
2.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619764914
@strongduanmu
I got your point. This RANGE algorithm is flexible for sharding, which is supposed to be the right choice for users, so please review my comment to perfect this PR. :)
Besides, we can consider another type of RANGE algorithm. 😀
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] SanmuWangZJU commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
SanmuWangZJU commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415479080
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
Review comment:
for line 61 and 73:
`doSharding()` method will be called while route value, it may be not efficient to build partitionRangeMap in `doSharding` method, I think it can be used as a static field and built once while initing instance.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620562298
> @strongduanmu That is exactly what I am thinking, maybe another PR to rename it as `partition.ranges` is expected. :)
> @strongduanmu That is exactly what I am thinking, maybe another PR to rename it as `partition.ranges` is expected. :)
I have modified it, help me review it, thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] coveralls edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619573975
## Pull Request Test Coverage Report for [Build 11315](https://coveralls.io/builds/30373187)
* **36** of **39** **(92.31%)** changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.07%**) to **57.2%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java](https://coveralls.io/builds/30373187/source?filename=sharding-core%2Fsharding-core-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fcore%2Fstrategy%2Falgorithm%2Fsharding%2FRangeShardingAlgorithm.java#L71) | 36 | 39 | 92.31%
<!-- | **Total:** | **36** | **39** | **92.31%** | -->
| Totals | [](https://coveralls.io/builds/30373187) |
| :-- | --: |
| Change from base [Build 11312](https://coveralls.io/builds/30368502): | 0.07% |
| Covered Lines: | 11901 |
| Relevant Lines: | 20806 |
---
##### 💛 - [Coveralls](https://coveralls.io)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620355296
@strongduanmu Hi
@terrymanu gave another option, i.e, `partition.range`. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620377881
> @strongduanmu Hi
> @terrymanu gave another option, i.e, `partition.range`. What do you think?
The parameter `partition.range` is better and more easier for users to understand!👍
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415823501
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
Review comment:
> for line 61 and 73:
> `doSharding()` method will be called while route value, it may be not efficient to build partitionRangeMap in `doSharding` method, I think it can be used as a static field and built once while initing instance.
@SanmuWangZJU @tristaZero
Hi, I have tried to use static field to define `partitionRangeMap`, but I found that it is not suitable. Users may instantiate multiple different range partition rules in the same application instance to correspond to different requirements. Static initialization means same range partition rule. Perhaps the instance field is more suitable.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620445075
@strongduanmu That is exactly what I am thinking, maybe another PR to rename it as `partition.ranges` is expected. :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620431368
> After more consideration, the second one `init()` is not necessary, maybe.
> At first, I considered to initialize `partitionRangeMap` in init(). However, that means `partitionRangeMap` will not be a final member. If so, at the multi-thread condition, it is not safe, which is more important than the performance of `doSharding()`.
> What's your idea about this one? Wait for your kind reply.
>
> Trista
Hi, @tristaZero , I agree with you that thread safety is more important. We can use another way to improve performance by providing a `useCache` parameter.
If users configured fewer partitions, the performance should be able to meet the requirements. But if the num of partition ranges is very large, such as 100 partitions, users can set `useCache` parameter to `true`, use the cache to improve performance, just like SQLParseResultCache.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r416290432
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import lombok.Getter;
+import lombok.Setter;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
Review comment:
Do you think there is any better names for `range.partition.split.value` to make users easy get its meaning. Like `range.partition.upper.points` or `range.partition.boundary.points`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619679212
@strongduanmu
Hi, I am glad to read your organized and straightforward code. :)
There is a concern about `range.partition.split.value` which is the key point of this algorithm.
Users need to list all endpoints of each range partitions, i.e, 10,20,30,40...
Besides, it generates complicated code handling. Do you think it is better to prefer the partition volume than listing all endpoints? That is, in this case: instead of `range.partition.split.value=10,20,30,40`, maybe we can consider `partition.volume = 10`. I am not sure, so I'd like to listen to your voice. 😀
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620421405
> How about `range.partitions`?
> @strongduanmu
> How about `range.partitions`?
> @strongduanmu
Hi, I found that the `partition.seconds` parameter is defined in the DatetimeShardingAlgorithm class,
Can we define it as `partition.ranges` in the RangeShardingAlgorithm class?
Ranges can remind users that this algorithm supports configuring multiple ranges for sharding.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620562298
> @strongduanmu That is exactly what I am thinking, maybe another PR to rename it as `partition.ranges` is expected. :)
I have modified it, help me review it, thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620346921
@strongduanmu Got it. Maybe we need a init() interface function to initialize some local members so as to avoid calculating some local values like partitionRangeMap
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415569806
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ Collection<String> result = new LinkedHashSet<>(availableTargetNames.size());
+ for (long value = shardingValue.getValueRange().lowerEndpoint(); value <= shardingValue.getValueRange().upperEndpoint(); value++) {
Review comment:
> It is likely that`value++` is time-consuming if there is a vast range.
> Here are some of my ideas.
> For instance,the shardings are as follows,
> 1,2 | 3,4,5 | 6,7,8,9 | 10,11
> So the `range.partition.split.value` is `2,5,9,11`
> Imaging the query condition is `2 =< x <= 9`, then we just need to calcute 2==2 and 9<11, therefore the results is `0,1,2`.
> That is, we can get the critical values from `range.partition.split.value`, so we only need to compare the bottom point and the top point with those critical values.
This suggestion is great, and I will optimize it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619764914
@strongduanmu
I got your point. This RANGE algorithm is flexible for sharding, which is supposed to be the right choice for users, so please review my comment to perfect this PR.
Besides, we can consider another type of RANGE algorithm. 😉
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] codecov-io commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619573969
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/5327?src=pr&el=h1) Report
> Merging [#5327](https://codecov.io/gh/apache/shardingsphere/pull/5327?src=pr&el=desc) into [master](https://codecov.io/gh/apache/shardingsphere/commit/d2a88c2212f4d8206232831f47e43dc72eb7b5de&el=desc) will **increase** coverage by `0.05%`.
> The diff coverage is `82.50%`.
[](https://codecov.io/gh/apache/shardingsphere/pull/5327?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5327 +/- ##
============================================
+ Coverage 53.63% 53.68% +0.05%
- Complexity 412 413 +1
============================================
Files 1158 1159 +1
Lines 20749 20789 +40
Branches 3751 3761 +10
============================================
+ Hits 11128 11161 +33
- Misses 8906 8911 +5
- Partials 715 717 +2
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/5327?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...egy/algorithm/sharding/RangeShardingAlgorithm.java](https://codecov.io/gh/apache/shardingsphere/pull/5327/diff?src=pr&el=tree#diff-c2hhcmRpbmctY29yZS9zaGFyZGluZy1jb3JlLWNvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvY29yZS9zdHJhdGVneS9hbGdvcml0aG0vc2hhcmRpbmcvUmFuZ2VTaGFyZGluZ0FsZ29yaXRobS5qYXZh) | `82.50% <82.50%> (ø)` | `1.00 <1.00> (?)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/5327?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/5327?src=pr&el=footer). Last update [d2a88c2...fa505b2](https://codecov.io/gh/apache/shardingsphere/pull/5327?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r416287924
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
Review comment:
@strongduanmu Got it. Maybe we need a `init()` interface function to initialize some local members so as to avoid calculating some local values like `partitionRangeMap`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415533641
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
Review comment:
@SanmuWangZJU Thanks. :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620350166
Hi @strongduanmu It seems this PR is excellent, so I can not wait to merge it, thanks for your effort! However, there are still some questions that remained for us,
1. Are there any better names for `range.partition.split.value` to make users easily get its meaning. Like `range.partition.upper.points` or `range.partition.boundary.points`?
2. Maybe a init() interface function is required to initialize some local members so as to avoid calculating some local values like partitionRangeMap
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415513252
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
Review comment:
Good suggestion, I will optimize this question.
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
Review comment:
Good suggestion, I will optimize this question.
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
Review comment:
> for line 61 and 73:
> `doSharding()` method will be called while route value, it may be not efficient to build partitionRangeMap in `doSharding` method, I think it can be used as a static field and built once while initing instance.
Good suggestion, I will optimize this question.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620431368
> After more consideration, the second one `init()` is not necessary, maybe.
> At first, I considered to initialize `partitionRangeMap` in init(). However, that means `partitionRangeMap` will not be a final member. If so, at the multi-thread condition, it is not safe, which is more important than the performance of `doSharding()`.
> What's your idea about this one? Wait for your kind reply.
>
> Trista
Hi, @tristaZero , I agree with you that thread safety is more important. We can use another way to improve performance by providing a `useCache` parameter.
If users configured fewer partitions, the performance should be able to meet the requirements. But if the num of partition ranges is very large,
such as 100 partitions, users can set `useCache` parameter to `true`, use the cache to improve performance, just like SQLParseResultCache.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on a change in pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#discussion_r415567013
##########
File path: sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java
##########
@@ -0,0 +1,128 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.shardingsphere.core.strategy.algorithm.sharding;
+
+import com.google.common.base.Preconditions;
+import com.google.common.base.Splitter;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Range;
+import com.google.common.primitives.Longs;
+import org.apache.commons.collections4.CollectionUtils;
+import org.apache.shardingsphere.api.sharding.standard.PreciseShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.RangeShardingValue;
+import org.apache.shardingsphere.api.sharding.standard.StandardShardingAlgorithm;
+
+import java.util.Collection;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Properties;
+import java.util.stream.Collectors;
+
+/**
+ * Range sharding algorithm.
+ * <p>
+ * Range sharding algorithm is similar to the rule of partition table.
+ * User can specify the range by setting the `range.partition.split.value` parameter.
+ * The `range.partition.split.value` parameter is an ordered list of numbers, separated by commas.
+ * </p>
+ * <p>
+ * For example: If the `range.partition.split.value` parameter is set to `1,5,10`,
+ * the parameter will split all values into four intervals——(-∞, 1), [1,5), [5,10), [10, +∞),
+ * which corresponding to partition_0, partition_1, partition_2, partition_3.
+ * The sharding values will be divided into different partition by its value.
+ * </p>
+ */
+public final class RangeShardingAlgorithm implements StandardShardingAlgorithm<Long> {
+
+ private static final String RANGE_PARTITION_SPLIT_VALUE = "range.partition.split.value";
+
+ private Properties properties = new Properties();
+
+ @Override
+ public String doSharding(final Collection<String> availableTargetNames, final PreciseShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, shardingValue.getValue()))) {
+ return each;
+ }
+ }
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Collection<String> doSharding(final Collection<String> availableTargetNames, final RangeShardingValue<Long> shardingValue) {
+ Preconditions.checkNotNull(properties.get(RANGE_PARTITION_SPLIT_VALUE), "Range sharding algorithm range partition split value cannot be null.");
+ Map<Integer, Range<Long>> partitionRangeMap = getPartitionRangeMap();
+ Collection<String> result = new LinkedHashSet<>(availableTargetNames.size());
+ for (long value = shardingValue.getValueRange().lowerEndpoint(); value <= shardingValue.getValueRange().upperEndpoint(); value++) {
+ for (String each : availableTargetNames) {
+ if (each.endsWith(getPartition(partitionRangeMap, value))) {
+ result.add(each);
+ }
+ }
+ }
+ return result;
+ }
+
+ private Map<Integer, Range<Long>> getPartitionRangeMap() {
+ List<Long> splitValues = Splitter.on(",").trimResults().splitToList(properties.get(RANGE_PARTITION_SPLIT_VALUE).toString())
+ .stream().map(Longs::tryParse).filter(Objects::nonNull).sorted().collect(Collectors.toList());
+ Preconditions.checkArgument(CollectionUtils.isNotEmpty(splitValues), "Range sharding algorithm range partition split value is not valid.");
+ Map<Integer, Range<Long>> partitionRangeMap = Maps.newHashMapWithExpectedSize(splitValues.size() + 1);
+ for (int i = 0; i < splitValues.size(); i++) {
+ Long splitValue = splitValues.get(i);
+ if (i == 0) {
+ partitionRangeMap.put(i, Range.lessThan(splitValue));
+ } else {
+ Long previousSplitValue = splitValues.get(i - 1);
+ partitionRangeMap.put(i, Range.closedOpen(previousSplitValue, splitValue));
+ }
+ if (i == splitValues.size() - 1) {
+ partitionRangeMap.put(i + 1, Range.atLeast(splitValue));
+ }
+ }
+ return partitionRangeMap;
+ }
+
+ private String getPartition(final Map<Integer, Range<Long>> partitionRangeMap, final Long value) {
+ for (Map.Entry<Integer, Range<Long>> entry : partitionRangeMap.entrySet()) {
+ if (entry.getValue().contains(value)) {
+ return entry.getKey().toString();
+ }
+ }
+ return partitionRangeMap.keySet().stream().mapToInt(Integer::valueOf).max().toString();
+ }
+
+ @Override
+ public String getType() {
+ return "RANGE";
+ }
+
+ @Override
Review comment:
Thank you, i will modify it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620350166
Hi @strongduanmu It seems this PR is excellent, so I can not wait to merge it, thanks for your effort! However, there are still some questions that remained for us,
1. Are there any better names for range.partition.split.value to make users easily get its meaning. Like range.partition.upper.points or range.partition.boundary.points?
2. Maybe a init() interface function is required to initialize some local members so as to avoid calculating some local values like partitionRangeMap
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] coveralls commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619573975
## Pull Request Test Coverage Report for [Build 11269](https://coveralls.io/builds/30349705)
* **35** of **40** **(87.5%)** changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.06%**) to **57.199%**
---
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
| :-----|--------------|--------|---: |
| [sharding-core/sharding-core-common/src/main/java/org/apache/shardingsphere/core/strategy/algorithm/sharding/RangeShardingAlgorithm.java](https://coveralls.io/builds/30349705/source?filename=sharding-core%2Fsharding-core-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fcore%2Fstrategy%2Falgorithm%2Fsharding%2FRangeShardingAlgorithm.java#L67) | 35 | 40 | 87.5%
<!-- | **Total:** | **35** | **40** | **87.5%** | -->
| Totals | [](https://coveralls.io/builds/30349705) |
| :-- | --: |
| Change from base [Build 11268](https://coveralls.io/builds/30347152): | 0.06% |
| Covered Lines: | 11891 |
| Relevant Lines: | 20789 |
---
##### 💛 - [Coveralls](https://coveralls.io)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero removed a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero removed a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620346921
@strongduanmu Got it. Maybe we need a init() interface function to initialize some local members so as to avoid calculating some local values like partitionRangeMap
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] strongduanmu edited a comment on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
strongduanmu edited a comment on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-620431368
> After more consideration, the second one `init()` is not necessary, maybe.
> At first, I considered to initialize `partitionRangeMap` in init(). However, that means `partitionRangeMap` will not be a final member. If so, at the multi-thread condition, it is not safe, which is more important than the performance of `doSharding()`.
> What's your idea about this one? Wait for your kind reply.
>
> Trista
Hi, @tristaZero , I agree with you that thread safety is more important.
We can use another way to improve performance by providing a `useCache` parameter. If users configured fewer partitions, the performance should be able to meet the requirements. But if the num of partition ranges is very large, such as 100 partitions, users can set `useCache` parameter to `true`, use the cache to improve performance, just like SQLParseResultCache.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [shardingsphere] tristaZero commented on pull request #5327: create RANGE sharding algorithm
Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #5327:
URL: https://github.com/apache/shardingsphere/pull/5327#issuecomment-619680926
BTW, instead of `Fixes #5280 .` which makes this issue closed after merging, `Ref #5280` is suggested, as we still welcome any other great ideas. :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org