You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Pochatkin (via GitHub)" <gi...@apache.org> on 2024/04/26 12:06:38 UTC
[PR] IGNITE-22065 Introduce partition public API [ignite-3]
Pochatkin opened a new pull request, #3673:
URL: https://github.com/apache/ignite-3/pull/3673
https://issues.apache.org/jira/browse/IGNITE-22065
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593694559
##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -141,6 +143,11 @@ public InternalTable internalTable() {
return tbl;
}
+ @Override
+ public <T extends Partition> PartitionManager<T> partitionManager() {
+ return (PartitionManager<T>) new HashPartitionManagerImpl(tbl, schemaReg, marshallers);
Review Comment:
We will generify Table interface with partition type and refactor this method
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593464625
##########
modules/api/src/main/java/org/apache/ignite/table/partition/Partition.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.ignite.table.partition;
+
+/**
+ * Marker interface which represents a partition reference for criteria query.
Review Comment:
Incorrect javadoc: criteria query mentioned.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/partition/HashPartitionManagerImpl.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.ignite.internal.table.partition;
+
+import static java.util.concurrent.CompletableFuture.allOf;
+import static java.util.concurrent.CompletableFuture.completedFuture;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.marshaller.MarshallerException;
+import org.apache.ignite.internal.marshaller.MarshallersProvider;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.schema.SchemaRegistry;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerException;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl;
+import org.apache.ignite.internal.schema.marshaller.reflection.KvMarshallerImpl;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.InternalTable;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+import org.apache.ignite.table.partition.HashPartition;
+import org.apache.ignite.table.partition.PartitionManager;
+
+/**
+ * Implementation of {@link PartitionManager} for tables with hash partitions.
+ */
+public class HashPartitionManagerImpl implements PartitionManager<HashPartition> {
+ private final InternalTable table;
+
+ private final SchemaRegistry schemaReg;
+
+ private final MarshallersProvider marshallers;
+
+ /**
+ * Constructor.
+ *
+ * @param table Internal table.
+ * @param schemaReg Schema registry.
+ * @param marshallers Marshallers.
+ */
+ public HashPartitionManagerImpl(
+ InternalTable table,
+ SchemaRegistry schemaReg,
+ MarshallersProvider marshallers
+ ) {
+ this.table = table;
+ this.schemaReg = schemaReg;
+ this.marshallers = marshallers;
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> primaryReplicaAsync(HashPartition partition) {
+ return table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ @Override
+ public CompletableFuture<Map<HashPartition, ClusterNode>> primaryReplicasAsync() {
+ HashPartition[] allPartitions = IntStream.range(0, table.partitions())
+ .mapToObj(HashPartition::new)
+ .toArray(HashPartition[]::new);
+
+ CompletableFuture<?>[] futures = new CompletableFuture<?>[allPartitions.length];
+ for (int i = 0; i < allPartitions.length; i++) {
+ HashPartition partition = allPartitions[i];
+ futures[i] = table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ return allOf(futures)
+ .thenApply(unused -> {
+ Map<HashPartition, ClusterNode> result = new HashMap<>();
+ for (int i = 0; i < allPartitions.length; i++) {
+ result.put(allPartitions[i], (ClusterNode) futures[i].join());
+ }
+ return result;
+ });
Review Comment:
* Avoid streams
* Avoid unnecessary `allPartitions` array
* Initialize collections with capacity when possible
```suggestion
int partitions = table.partitions();
CompletableFuture<?>[] futures = new CompletableFuture<?>[partitions];
for (int i = 0; i < partitions; i++) {
futures[i] = table.partitionLocation(new TablePartitionId(table.tableId(), i));
}
return allOf(futures)
.thenApply(unused -> {
Map<HashPartition, ClusterNode> result = new HashMap<>(partitions);
for (int i = 0; i < partitions; i++) {
result.put(new HashPartition(i), (ClusterNode) futures[i].join());
}
return result;
});
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -141,6 +143,11 @@ public InternalTable internalTable() {
return tbl;
}
+ @Override
+ public <T extends Partition> PartitionManager<T> partitionManager() {
+ return (PartitionManager<T>) new HashPartitionManagerImpl(tbl, schemaReg, marshallers);
Review Comment:
How is it going to work with `RangePartition`?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/partition/HashPartitionManagerImpl.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.ignite.internal.table.partition;
+
+import static java.util.concurrent.CompletableFuture.allOf;
+import static java.util.concurrent.CompletableFuture.completedFuture;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.marshaller.MarshallerException;
+import org.apache.ignite.internal.marshaller.MarshallersProvider;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.schema.SchemaRegistry;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerException;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl;
+import org.apache.ignite.internal.schema.marshaller.reflection.KvMarshallerImpl;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.InternalTable;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+import org.apache.ignite.table.partition.HashPartition;
+import org.apache.ignite.table.partition.PartitionManager;
+
+/**
+ * Implementation of {@link PartitionManager} for tables with hash partitions.
+ */
+public class HashPartitionManagerImpl implements PartitionManager<HashPartition> {
+ private final InternalTable table;
+
+ private final SchemaRegistry schemaReg;
+
+ private final MarshallersProvider marshallers;
+
+ /**
+ * Constructor.
+ *
+ * @param table Internal table.
+ * @param schemaReg Schema registry.
+ * @param marshallers Marshallers.
+ */
+ public HashPartitionManagerImpl(
+ InternalTable table,
+ SchemaRegistry schemaReg,
+ MarshallersProvider marshallers
+ ) {
+ this.table = table;
+ this.schemaReg = schemaReg;
+ this.marshallers = marshallers;
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> primaryReplicaAsync(HashPartition partition) {
+ return table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ @Override
+ public CompletableFuture<Map<HashPartition, ClusterNode>> primaryReplicasAsync() {
+ HashPartition[] allPartitions = IntStream.range(0, table.partitions())
+ .mapToObj(HashPartition::new)
+ .toArray(HashPartition[]::new);
+
+ CompletableFuture<?>[] futures = new CompletableFuture<?>[allPartitions.length];
+ for (int i = 0; i < allPartitions.length; i++) {
+ HashPartition partition = allPartitions[i];
+ futures[i] = table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ return allOf(futures)
+ .thenApply(unused -> {
+ Map<HashPartition, ClusterNode> result = new HashMap<>();
+ for (int i = 0; i < allPartitions.length; i++) {
+ result.put(allPartitions[i], (ClusterNode) futures[i].join());
+ }
+ return result;
+ });
+ }
+
+ @Override
+ public <K> CompletableFuture<HashPartition> partitionAsync(K key, Mapper<K> mapper) {
+ Objects.requireNonNull(key);
+ Objects.requireNonNull(mapper);
+
+ BinaryRowEx keyRow;
Review Comment:
Can be moved to inner scope and combined with initialization.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1587532341
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
Review Comment:
`CompletableFuture<ClusterNode> mapToNode(T partition);` ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1592162164
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
Ok, and we'll be using partitions identified by `int` anyway.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1596568312
##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -141,6 +142,11 @@ public InternalTable internalTable() {
return tbl;
}
+ @Override
+ public PartitionManager partitionManager() {
+ return new HashPartitionManagerImpl(tbl, schemaReg, marshallers);
Review Comment:
No, schemaReg is mutable
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1589294361
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key tuple.
+ * @return Partition instance which contains provided key.
+ */
+ CompletableFuture<T> partitionFromKeyAsync(Tuple key);
+}
Review Comment:
How this is not needed ? This is required for reads from replicas.
For example, I would like to map a read-only job to some replica.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1592102192
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
We will implement similar functionality in future
https://dev.mysql.com/doc/mysql-partitioning-excerpt/8.3/en/partitioning-range.html
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1584475750
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java:
##########
@@ -793,14 +788,10 @@ private <R> CompletableFuture<R> evaluateReadOnlyPrimaryNode(
CompletableFuture<R> fut = primaryReplicaFuture.thenCompose(primaryReplica -> {
try {
- ClusterNode node = clusterNodeResolver.getByConsistentId(primaryReplica.getLeaseholder());
-
- if (node == null) {
- throw new TransactionException(REPLICA_UNAVAILABLE_ERR, "Failed to resolve the primary replica node [consistentId="
- + primaryReplica.getLeaseholder() + ']');
- }
+ ClusterNode node = getClusterNode(primaryReplica.getLeaseholder());
- return replicaSvc.invoke(node, op.apply(tablePartitionId, primaryReplica.getStartTime().longValue()));
+ return replicaSvc.invoke(node, op.apply(tablePartitionId, primaryReplica.getStartTime().longValue())
+ );
Review Comment:
```suggestion
return replicaSvc.invoke(node, op.apply(tablePartitionId, primaryReplica.getStartTime().longValue()));
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1587532341
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
Review Comment:
`CompletableFuture<ClusterNode> mapPrimaryToNodeAsync(T partition);` ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593756155
##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -141,6 +143,11 @@ public InternalTable internalTable() {
return tbl;
}
+ @Override
+ public <T extends Partition> PartitionManager<T> partitionManager() {
+ return (PartitionManager<T>) new HashPartitionManagerImpl(tbl, schemaReg, marshallers);
Review Comment:
As discussed in private, we don't need to generify `Table`, and this method will have a `Class<T>` parameter to validate the partition type.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593461675
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
Ok, we should keep the classes then.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1592327518
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
Range partition is identfied by (lower, upper) bounds.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1592519706
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Partition.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.ignite.table.criteria;
Review Comment:
Done
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593721812
##########
modules/api/src/main/java/org/apache/ignite/table/Table.java:
##########
@@ -37,6 +39,13 @@ public interface Table {
*/
String name();
+ /**
+ * Gets a partition manager of a table.
+ *
+ * @return Partition manager.
+ */
+ <T extends Partition> PartitionManager<T> partitionManager();
Review Comment:
The generic part does not make sense to me:
* As a user, how am I supposed to know which partition type will be returned?
* As an implementer, with type erasure there is no way to know which partition type the user expects.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1591790330
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Partition.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.ignite.table.criteria;
Review Comment:
We should not mix up Criteria API here. Partition management belongs to `table` package and should not extend `Criteria`.
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> mapPrimaryToNodeAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
Review Comment:
```suggestion
CompletableFuture<Map<T, ClusterNode>> primaryReplicasAsync();
```
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> mapPrimaryToNodeAsync(T partition);
Review Comment:
```suggestion
CompletableFuture<ClusterNode> primaryReplicaAsync(T partition);
```
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> mapPrimaryToNodeAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
Review Comment:
```suggestion
<K> CompletableFuture<T> partitionAsync(K key, Mapper<K> mapper);
```
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> mapPrimaryToNodeAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key tuple.
+ * @return Partition instance which contains provided key.
+ */
+ CompletableFuture<T> partitionFromKeyAsync(Tuple key);
Review Comment:
```suggestion
CompletableFuture<T> partitionAsync(Tuple key);
```
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
Do we actually need a class to wrap an integer? The API would be much simpler with `int` / `Integer` without all the generics for partition type.
Do we expect other types of partitions in the API?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1589294361
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key tuple.
+ * @return Partition instance which contains provided key.
+ */
+ CompletableFuture<T> partitionFromKeyAsync(Tuple key);
+}
Review Comment:
How this is not needed ? This is required for reads from replicas.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "valepakh (via GitHub)" <gi...@apache.org>.
valepakh commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1581219208
##########
modules/table/src/main/java/org/apache/ignite/internal/table/partition/HashPartitionManagerImpl.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.ignite.internal.table.partition;
+
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.lang.IgniteInternalException;
+import org.apache.ignite.internal.marshaller.MarshallerException;
+import org.apache.ignite.internal.marshaller.MarshallersProvider;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.schema.SchemaRegistry;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerException;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl;
+import org.apache.ignite.internal.schema.marshaller.reflection.KvMarshallerImpl;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.InternalTable;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+import org.apache.ignite.table.partition.HashPartition;
+import org.apache.ignite.table.partition.PartitionManager;
+
+/**
+ * Implementation of {@link PartitionManager} for tables with hash partitions.
+ */
+public class HashPartitionManagerImpl implements PartitionManager<HashPartition> {
+ private final InternalTable table;
+
+ private final SchemaRegistry schemaReg;
+
+ private final MarshallersProvider marshallers;
+
+ /**
+ * Constructor.
+ *
+ * @param table Internal table.
+ * @param schemaReg Schema registry.
+ * @param marshallers Marshallers.
+ */
+ public HashPartitionManagerImpl(
+ InternalTable table,
+ SchemaRegistry schemaReg,
+ MarshallersProvider marshallers
+ ) {
+ this.table = table;
+ this.schemaReg = schemaReg;
+ this.marshallers = marshallers;
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> partitionLocationAsync(HashPartition partition) {
+ return table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ @Override
+ public CompletableFuture<Map<HashPartition, ClusterNode>> allPartitionsAsync() {
+ Map<HashPartition, ClusterNode> resultMap = new ConcurrentHashMap<>();
Review Comment:
You could remove the need for concurrent container by iterating on both arrays after the `allOf`
##########
modules/table/src/main/java/org/apache/ignite/internal/table/partition/HashPartitionManagerImpl.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.ignite.internal.table.partition;
+
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.lang.IgniteInternalException;
+import org.apache.ignite.internal.marshaller.MarshallerException;
+import org.apache.ignite.internal.marshaller.MarshallersProvider;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.schema.SchemaRegistry;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerException;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl;
+import org.apache.ignite.internal.schema.marshaller.reflection.KvMarshallerImpl;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.InternalTable;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+import org.apache.ignite.table.partition.HashPartition;
+import org.apache.ignite.table.partition.PartitionManager;
+
+/**
+ * Implementation of {@link PartitionManager} for tables with hash partitions.
+ */
+public class HashPartitionManagerImpl implements PartitionManager<HashPartition> {
+ private final InternalTable table;
+
+ private final SchemaRegistry schemaReg;
+
+ private final MarshallersProvider marshallers;
+
+ /**
+ * Constructor.
+ *
+ * @param table Internal table.
+ * @param schemaReg Schema registry.
+ * @param marshallers Marshallers.
+ */
+ public HashPartitionManagerImpl(
+ InternalTable table,
+ SchemaRegistry schemaReg,
+ MarshallersProvider marshallers
+ ) {
+ this.table = table;
+ this.schemaReg = schemaReg;
+ this.marshallers = marshallers;
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> partitionLocationAsync(HashPartition partition) {
+ return table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ @Override
+ public CompletableFuture<Map<HashPartition, ClusterNode>> allPartitionsAsync() {
+ Map<HashPartition, ClusterNode> resultMap = new ConcurrentHashMap<>();
+
+ HashPartition[] allPartitions = IntStream.range(0, table.partitions())
+ .mapToObj(HashPartition::new)
+ .toArray(HashPartition[]::new);
+
+ CompletableFuture<?>[] futures = new CompletableFuture<?>[allPartitions.length];
+ for (int i = 0; i < allPartitions.length; i++) {
+ HashPartition partition = allPartitions[i];
+ futures[i] = table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()))
+ .thenAccept(node -> resultMap.put(partition, node));
+ }
+
+ return CompletableFuture.allOf(futures)
+ .thenApply(unused -> resultMap);
+ }
+
+ @Override
+ public <K> CompletableFuture<HashPartition> partitionFromKeyAsync(K key, Mapper<K> mapper) {
+ Objects.requireNonNull(key);
+ Objects.requireNonNull(mapper);
+
+ BinaryRowEx keyRow;
+ var marshaller = new KvMarshallerImpl<>(schemaReg.lastKnownSchema(), marshallers, mapper, mapper);
+ try {
+ keyRow = marshaller.marshal(key);
+ } catch (MarshallerException e) {
+ throw new IgniteInternalException("Cannot marshal key", e);
Review Comment:
Deprecated constructors.
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * Then interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where located primary replica of provided partition.
Review Comment:
```suggestion
* @return Cluster node where primary replica of provided partition is located.
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/partition/HashPartitionManagerImpl.java:
##########
@@ -0,0 +1,123 @@
+/*
+ * 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.ignite.internal.table.partition;
+
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.lang.IgniteInternalException;
+import org.apache.ignite.internal.marshaller.MarshallerException;
+import org.apache.ignite.internal.marshaller.MarshallersProvider;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.schema.SchemaRegistry;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerException;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl;
+import org.apache.ignite.internal.schema.marshaller.reflection.KvMarshallerImpl;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.InternalTable;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+import org.apache.ignite.table.partition.HashPartition;
+import org.apache.ignite.table.partition.PartitionManager;
+
+/**
+ * Implementation of {@link PartitionManager} for tables with hash partitions.
+ */
+public class HashPartitionManagerImpl implements PartitionManager<HashPartition> {
+ private final InternalTable table;
+
+ private final SchemaRegistry schemaReg;
+
+ private final MarshallersProvider marshallers;
+
+ /**
+ * Constructor.
+ *
+ * @param table Internal table.
+ * @param schemaReg Schema registry.
+ * @param marshallers Marshallers.
+ */
+ public HashPartitionManagerImpl(
+ InternalTable table,
+ SchemaRegistry schemaReg,
+ MarshallersProvider marshallers
+ ) {
+ this.table = table;
+ this.schemaReg = schemaReg;
+ this.marshallers = marshallers;
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> partitionLocationAsync(HashPartition partition) {
+ return table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ @Override
+ public CompletableFuture<Map<HashPartition, ClusterNode>> allPartitionsAsync() {
+ Map<HashPartition, ClusterNode> resultMap = new ConcurrentHashMap<>();
+
+ HashPartition[] allPartitions = IntStream.range(0, table.partitions())
+ .mapToObj(HashPartition::new)
+ .toArray(HashPartition[]::new);
+
+ CompletableFuture<?>[] futures = new CompletableFuture<?>[allPartitions.length];
+ for (int i = 0; i < allPartitions.length; i++) {
+ HashPartition partition = allPartitions[i];
+ futures[i] = table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()))
+ .thenAccept(node -> resultMap.put(partition, node));
+ }
+
+ return CompletableFuture.allOf(futures)
+ .thenApply(unused -> resultMap);
+ }
+
+ @Override
+ public <K> CompletableFuture<HashPartition> partitionFromKeyAsync(K key, Mapper<K> mapper) {
+ Objects.requireNonNull(key);
+ Objects.requireNonNull(mapper);
+
+ BinaryRowEx keyRow;
+ var marshaller = new KvMarshallerImpl<>(schemaReg.lastKnownSchema(), marshallers, mapper, mapper);
+ try {
+ keyRow = marshaller.marshal(key);
+ } catch (MarshallerException e) {
+ throw new IgniteInternalException("Cannot marshal key", e);
+ }
+
+ return CompletableFuture.completedFuture(new HashPartition(table.partition(keyRow)));
Review Comment:
`keyRow` declaration and return could be moved inside the try block as in the next method. Also static import would be nice.
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Criteria.java:
##########
@@ -69,6 +69,23 @@ static Expression columnValue(String columnName, Condition condition) {
return new Expression(condition.getOperator(), newElements);
}
+ /**
+ * Creates a predicate that test whether the partition value is supply to the give condition.
Review Comment:
Could you rephrase that - I don't understand what does this mean.
##########
modules/core/src/main/java/org/apache/ignite/internal/table/criteria/SqlSerializer.java:
##########
@@ -124,6 +125,11 @@ public <T> void visit(Expression expression, @Nullable Void context) {
}
}
+ @Override
+ public void visit(Partition partition, @Nullable Void context) {
+ throw new UnsupportedOperationException("This operation doesn't implemented yet.");
Review Comment:
Could you please create tickets for implementing this if needed?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java:
##########
@@ -1846,10 +1837,26 @@ protected CompletableFuture<IgniteBiTuple<ClusterNode, Long>> enlist(int partId,
TablePartitionId tablePartitionId = new TablePartitionId(tableId, partId);
tx.assignCommitPartition(tablePartitionId);
+ return partitionMeta(tablePartitionId).thenApply(meta -> {
+ TablePartitionId partGroupId = new TablePartitionId(tableId, partId);
+
+ return tx.enlist(partGroupId, new IgniteBiTuple<>(
+ getClusterNode(meta.getLeaseholder()),
+ meta.getStartTime().longValue())
+ );
+ });
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> partitionLocation(ReplicationGroupId partition) {
+ return partitionMeta(partition).thenApply(meta -> getClusterNode(meta.getLeaseholder()));
+ }
+
+ private CompletableFuture<ReplicaMeta> partitionMeta(ReplicationGroupId partition) {
HybridTimestamp now = clock.now();
CompletableFuture<ReplicaMeta> primaryReplicaFuture = placementDriver.awaitPrimaryReplica(
- tablePartitionId,
+ partition,
Review Comment:
Let's keep the previous name - `tablePartitionId`
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java:
##########
@@ -1858,20 +1865,22 @@ protected CompletableFuture<IgniteBiTuple<ClusterNode, Long>> enlist(int partId,
return primaryReplicaFuture.handle((primaryReplica, e) -> {
if (e != null) {
throw withCause(TransactionException::new, REPLICA_UNAVAILABLE_ERR, "Failed to get the primary replica"
- + " [tablePartitionId=" + tablePartitionId + ", awaitTimestamp=" + now + ']', e);
+ + " [tablePartitionId=" + partition + ", awaitTimestamp=" + now + ']', e);
}
- ClusterNode node = clusterNodeResolver.getByConsistentId(primaryReplica.getLeaseholder());
+ return primaryReplica;
+ });
+ }
- if (node == null) {
- throw new TransactionException(REPLICA_UNAVAILABLE_ERR, "Failed to resolve the primary replica node [consistentId="
- + primaryReplica.getLeaseholder() + ']');
- }
+ private ClusterNode getClusterNode(String leaserHolder) {
Review Comment:
```suggestion
private ClusterNode getClusterNode(@Nullable String leaserHolder) {
```
##########
modules/api/src/main/java/org/apache/ignite/table/criteria/Criteria.java:
##########
@@ -69,6 +69,23 @@ static Expression columnValue(String columnName, Condition condition) {
return new Expression(condition.getOperator(), newElements);
}
+ /**
+ * Creates a predicate that test whether the partition value is supply to the give condition.
+ *
+ * @param partition Partition value.
+ * @param condition Target condition.
+ * @return The created expression instance.
+ */
+ static Expression partitionValue(Partition partition, Condition condition) {
Review Comment:
This method is unused - probably should add a test that uses it.
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * Then interface can be used to get all partitions of a table,
Review Comment:
```suggestion
* This interface can be used to get all partitions of a table,
```
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * Then interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where located primary replica of provided partition.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where located primary replica of the partition.
Review Comment:
```suggestion
* @return Map from partition to cluster node where primary replica of the partition is located.
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java:
##########
@@ -793,14 +788,10 @@ private <R> CompletableFuture<R> evaluateReadOnlyPrimaryNode(
CompletableFuture<R> fut = primaryReplicaFuture.thenCompose(primaryReplica -> {
try {
- ClusterNode node = clusterNodeResolver.getByConsistentId(primaryReplica.getLeaseholder());
-
- if (node == null) {
- throw new TransactionException(REPLICA_UNAVAILABLE_ERR, "Failed to resolve the primary replica node [consistentId="
- + primaryReplica.getLeaseholder() + ']');
- }
-
- return replicaSvc.invoke(node, op.apply(tablePartitionId, primaryReplica.getStartTime().longValue()));
+ return replicaSvc.invoke(
+ getClusterNode(primaryReplica.getLeaseholder()),
+ op.apply(tablePartitionId, primaryReplica.getStartTime().longValue())
+ );
Review Comment:
```suggestion
ClusterNode node = getClusterNode(primaryReplica.getLeaseholder());
return replicaSvc.invoke(node, op.apply(tablePartitionId, primaryReplica.getStartTime().longValue()));
```
##########
modules/table/src/main/java/org/apache/ignite/internal/table/InternalTable.java:
##########
@@ -486,4 +487,12 @@ Publisher<BinaryRow> lookup(
* @return Streamer flush executor.
*/
ScheduledExecutorService streamerFlushExecutor();
+
+ /**
+ * Returns cluster node {@link ClusterNode} where located primary replica of replication group.
Review Comment:
```suggestion
* Returns {@link ClusterNode} where primary replica of replication group is located.
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1589099202
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
Review Comment:
For now this is not needed for MapReduce funtionality. So, we can create a ticket for this API
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1588795483
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
Review Comment:
Should we provide the API to subscribe for assignment change ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin merged PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1589098220
##########
modules/api/src/main/java/org/apache/ignite/table/Table.java:
##########
@@ -37,6 +39,13 @@ public interface Table {
*/
String name();
+ /**
+ * Gets a partition manager of a table.
+ *
+ * @return Partition manager.
+ */
+ PartitionManager<HashPartition> partitionManager();
Review Comment:
Done
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
Review Comment:
Done
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593774693
##########
modules/api/src/main/java/org/apache/ignite/table/Table.java:
##########
@@ -37,6 +39,13 @@ public interface Table {
*/
String name();
+ /**
+ * Gets a partition manager of a table.
+ *
+ * @return Partition manager.
+ */
+ <T extends Partition> PartitionManager<T> partitionManager();
Review Comment:
Alternatively:
* Make partition classes private
* Remove generics, use `Partition` interface everywhere
Looks like the user does not need to see the actual class, they just pass it around.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593713299
##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -141,6 +143,11 @@ public InternalTable internalTable() {
return tbl;
}
+ @Override
+ public <T extends Partition> PartitionManager<T> partitionManager() {
+ return (PartitionManager<T>) new HashPartitionManagerImpl(tbl, schemaReg, marshallers);
Review Comment:
It is going to be a breaking change, right? So we do some future-proofing with `Partition` interface, but not here?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1588796081
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key tuple.
+ * @return Partition instance which contains provided key.
+ */
+ CompletableFuture<T> partitionFromKeyAsync(Tuple key);
+}
Review Comment:
I think we also need a method to get primary and backups for key
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key tuple.
+ * @return Partition instance which contains provided key.
+ */
+ CompletableFuture<T> partitionFromKeyAsync(Tuple key);
+}
Review Comment:
I think we also need a method to get the primary and backups for key
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1587529254
##########
modules/api/src/main/java/org/apache/ignite/table/Table.java:
##########
@@ -37,6 +39,13 @@ public interface Table {
*/
String name();
+ /**
+ * Gets a partition manager of a table.
+ *
+ * @return Partition manager.
+ */
+ PartitionManager<HashPartition> partitionManager();
Review Comment:
`<T extends Partition> PartitionManager<T> partitionManager();`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1589294734
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
Review Comment:
Ok
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1589294361
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key tuple.
+ * @return Partition instance which contains provided key.
+ */
+ CompletableFuture<T> partitionFromKeyAsync(Tuple key);
+}
Review Comment:
How this is not needed ? This is required for reads from replicas.
For example, I could map a read-only job to some replica.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1587532341
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
Review Comment:
`CompletableFuture<ClusterNode> mapToNodeAsync(T partition);` ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1592089890
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
What is a range partition, do you have more details?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593712153
##########
modules/table/src/main/java/org/apache/ignite/internal/table/partition/HashPartitionManagerImpl.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.ignite.internal.table.partition;
+
+import static java.util.concurrent.CompletableFuture.allOf;
+import static java.util.concurrent.CompletableFuture.completedFuture;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.marshaller.MarshallerException;
+import org.apache.ignite.internal.marshaller.MarshallersProvider;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.schema.SchemaRegistry;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerException;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl;
+import org.apache.ignite.internal.schema.marshaller.reflection.KvMarshallerImpl;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.InternalTable;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+import org.apache.ignite.table.partition.HashPartition;
+import org.apache.ignite.table.partition.PartitionManager;
+
+/**
+ * Implementation of {@link PartitionManager} for tables with hash partitions.
+ */
+public class HashPartitionManagerImpl implements PartitionManager<HashPartition> {
+ private final InternalTable table;
+
+ private final SchemaRegistry schemaReg;
+
+ private final MarshallersProvider marshallers;
+
+ /**
+ * Constructor.
+ *
+ * @param table Internal table.
+ * @param schemaReg Schema registry.
+ * @param marshallers Marshallers.
+ */
+ public HashPartitionManagerImpl(
+ InternalTable table,
+ SchemaRegistry schemaReg,
+ MarshallersProvider marshallers
+ ) {
+ this.table = table;
+ this.schemaReg = schemaReg;
+ this.marshallers = marshallers;
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> primaryReplicaAsync(HashPartition partition) {
+ return table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ @Override
+ public CompletableFuture<Map<HashPartition, ClusterNode>> primaryReplicasAsync() {
+ HashPartition[] allPartitions = IntStream.range(0, table.partitions())
+ .mapToObj(HashPartition::new)
+ .toArray(HashPartition[]::new);
+
+ CompletableFuture<?>[] futures = new CompletableFuture<?>[allPartitions.length];
+ for (int i = 0; i < allPartitions.length; i++) {
+ HashPartition partition = allPartitions[i];
+ futures[i] = table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ return allOf(futures)
+ .thenApply(unused -> {
+ Map<HashPartition, ClusterNode> result = new HashMap<>();
+ for (int i = 0; i < allPartitions.length; i++) {
+ result.put(allPartitions[i], (ClusterNode) futures[i].join());
+ }
+ return result;
+ });
+ }
+
+ @Override
+ public <K> CompletableFuture<HashPartition> partitionAsync(K key, Mapper<K> mapper) {
+ Objects.requireNonNull(key);
+ Objects.requireNonNull(mapper);
+
+ BinaryRowEx keyRow;
Review Comment:
Done
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593755089
##########
modules/api/src/main/java/org/apache/ignite/table/Table.java:
##########
@@ -37,6 +39,13 @@ public interface Table {
*/
String name();
+ /**
+ * Gets a partition manager of a table.
+ *
+ * @return Partition manager.
+ */
+ <T extends Partition> PartitionManager<T> partitionManager();
Review Comment:
As discussed in private, let's add `Class<T> partitionClass` parameter to this method:
* The user is supposed to know the partition type, because it is a part of the table schema
* If the user makes a mistake, we can throw a meaningful exception right away (instead of a `ClassCastException` somewhere later)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1592067473
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
Yes, we expecting range partitions in future and this abstraction for future
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1593711858
##########
modules/api/src/main/java/org/apache/ignite/table/partition/Partition.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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.ignite.table.partition;
+
+/**
+ * Marker interface which represents a partition reference for criteria query.
Review Comment:
Done
##########
modules/table/src/main/java/org/apache/ignite/internal/table/partition/HashPartitionManagerImpl.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.ignite.internal.table.partition;
+
+import static java.util.concurrent.CompletableFuture.allOf;
+import static java.util.concurrent.CompletableFuture.completedFuture;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletableFuture;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.marshaller.MarshallerException;
+import org.apache.ignite.internal.marshaller.MarshallersProvider;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.internal.schema.BinaryRowEx;
+import org.apache.ignite.internal.schema.SchemaRegistry;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerException;
+import org.apache.ignite.internal.schema.marshaller.TupleMarshallerImpl;
+import org.apache.ignite.internal.schema.marshaller.reflection.KvMarshallerImpl;
+import org.apache.ignite.internal.schema.row.Row;
+import org.apache.ignite.internal.table.InternalTable;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+import org.apache.ignite.table.partition.HashPartition;
+import org.apache.ignite.table.partition.PartitionManager;
+
+/**
+ * Implementation of {@link PartitionManager} for tables with hash partitions.
+ */
+public class HashPartitionManagerImpl implements PartitionManager<HashPartition> {
+ private final InternalTable table;
+
+ private final SchemaRegistry schemaReg;
+
+ private final MarshallersProvider marshallers;
+
+ /**
+ * Constructor.
+ *
+ * @param table Internal table.
+ * @param schemaReg Schema registry.
+ * @param marshallers Marshallers.
+ */
+ public HashPartitionManagerImpl(
+ InternalTable table,
+ SchemaRegistry schemaReg,
+ MarshallersProvider marshallers
+ ) {
+ this.table = table;
+ this.schemaReg = schemaReg;
+ this.marshallers = marshallers;
+ }
+
+ @Override
+ public CompletableFuture<ClusterNode> primaryReplicaAsync(HashPartition partition) {
+ return table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ @Override
+ public CompletableFuture<Map<HashPartition, ClusterNode>> primaryReplicasAsync() {
+ HashPartition[] allPartitions = IntStream.range(0, table.partitions())
+ .mapToObj(HashPartition::new)
+ .toArray(HashPartition[]::new);
+
+ CompletableFuture<?>[] futures = new CompletableFuture<?>[allPartitions.length];
+ for (int i = 0; i < allPartitions.length; i++) {
+ HashPartition partition = allPartitions[i];
+ futures[i] = table.partitionLocation(new TablePartitionId(table.tableId(), partition.partitionId()));
+ }
+
+ return allOf(futures)
+ .thenApply(unused -> {
+ Map<HashPartition, ClusterNode> result = new HashMap<>();
+ for (int i = 0; i < allPartitions.length; i++) {
+ result.put(allPartitions[i], (ClusterNode) futures[i].join());
+ }
+ return result;
+ });
Review Comment:
Done
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1592367462
##########
modules/api/src/main/java/org/apache/ignite/table/partition/HashPartition.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.io.Serializable;
+import org.apache.ignite.table.criteria.CriteriaVisitor;
+import org.apache.ignite.table.criteria.Partition;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Hash partition representation.
+ */
+public class HashPartition implements Partition, Serializable {
+ private static final long serialVersionUID = 1717320056615864614L;
+
+ private final int partitionId;
Review Comment:
> Ok, and we'll be using partitions identified by `int` anyway.
Range partition requires two integers
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ptupitsyn (via GitHub)" <gi...@apache.org>.
ptupitsyn commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1594198560
##########
modules/table/src/main/java/org/apache/ignite/internal/table/TableImpl.java:
##########
@@ -141,6 +142,11 @@ public InternalTable internalTable() {
return tbl;
}
+ @Override
+ public PartitionManager partitionManager() {
+ return new HashPartitionManagerImpl(tbl, schemaReg, marshallers);
Review Comment:
Can we return a cached instance?
##########
modules/api/src/main/java/org/apache/ignite/lang/UnsupportedPartitionTypeException.java:
##########
@@ -0,0 +1,51 @@
+/*
+ * 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.ignite.lang;
+
+import static org.apache.ignite.lang.ErrorGroups.Table.UNSUPPORTED_PARTITION_TYPE_ERR;
+
+import java.util.UUID;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * This exception is thrown when a table is not supported required partition type.
Review Comment:
```suggestion
* This exception is thrown when a table does not support the specified partition type.
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "Pochatkin (via GitHub)" <gi...@apache.org>.
Pochatkin commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1589099721
##########
modules/api/src/main/java/org/apache/ignite/table/partition/PartitionManager.java:
##########
@@ -0,0 +1,67 @@
+/*
+ * 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.ignite.table.partition;
+
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.network.ClusterNode;
+import org.apache.ignite.table.Tuple;
+import org.apache.ignite.table.mapper.Mapper;
+
+/**
+ * The partition manager provides the ability to obtain information about table partitions.
+ * This interface can be used to get all partitions of a table,
+ * the location of the primary replica of a partition,
+ * the partition for a specific table key.
+ *
+ * @param <T> Partitioning type.
+ */
+public interface PartitionManager<T> {
+ /**
+ * Returns location of primary replica for provided partition.
+ *
+ * @param partition Partition instance.
+ * @return Cluster node where primary replica of provided partition is located.
+ */
+ CompletableFuture<ClusterNode> partitionLocationAsync(T partition);
+
+ /**
+ * Returns map with all partitions and their locations.
+ *
+ * @return Map from partition to cluster node where primary replica of the partition is located.
+ */
+ CompletableFuture<Map<T, ClusterNode>> allPartitionsAsync();
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key.
+ * @param mapper Table key mapper.
+ * @param <K> Key type.
+ * @return Partition instance which contains provided key.
+ */
+ <K> CompletableFuture<T> partitionFromKeyAsync(K key, Mapper<K> mapper);
+
+ /**
+ * Returns partition instance for provided table key.
+ *
+ * @param key Table key tuple.
+ * @return Partition instance which contains provided key.
+ */
+ CompletableFuture<T> partitionFromKeyAsync(Tuple key);
+}
Review Comment:
For now we don't need it in MapReduce, but we can create a ticket at least
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22065 Introduce partition public API [ignite-3]
Posted by "ascherbakoff (via GitHub)" <gi...@apache.org>.
ascherbakoff commented on code in PR #3673:
URL: https://github.com/apache/ignite-3/pull/3673#discussion_r1587529254
##########
modules/api/src/main/java/org/apache/ignite/table/Table.java:
##########
@@ -37,6 +39,13 @@ public interface Table {
*/
String name();
+ /**
+ * Gets a partition manager of a table.
+ *
+ * @return Partition manager.
+ */
+ PartitionManager<HashPartition> partitionManager();
Review Comment:
<T extends Partition>PartitionManager<T> partitionManager();
##########
modules/api/src/main/java/org/apache/ignite/table/Table.java:
##########
@@ -37,6 +39,13 @@ public interface Table {
*/
String name();
+ /**
+ * Gets a partition manager of a table.
+ *
+ * @return Partition manager.
+ */
+ PartitionManager<HashPartition> partitionManager();
Review Comment:
`<T extends Partition>PartitionManager<T> partitionManager();`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org