You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "sashapolo (via GitHub)" <gi...@apache.org> on 2023/02/03 10:54:34 UTC
[GitHub] [ignite-3] sashapolo opened a new pull request, #1626: IGNITE-14691 Implement reactive Meta Storage range methods
sashapolo opened a new pull request, #1626:
URL: https://github.com/apache/ignite-3/pull/1626
https://issues.apache.org/jira/browse/IGNITE-14691
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1098327976
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaManager.java:
##########
@@ -432,22 +433,48 @@ public void stop() throws Exception {
* @param tblId Table id.
* @return The latest schema version.
*/
+ // TODO: Make this method async, see https://issues.apache.org/jira/browse/IGNITE-18732
private int latestSchemaVersion(UUID tblId) {
- try (Cursor<Entry> cur = metastorageMgr.prefix(schemaHistPrefix(tblId))) {
- int lastVer = INITIAL_SCHEMA_VERSION;
+ var latestVersionFuture = new CompletableFuture<Integer>();
- for (Entry ent : cur) {
- String key = new String(ent.key(), StandardCharsets.UTF_8);
+ metastorageMgr.prefix(schemaHistPrefix(tblId)).subscribe(new Subscriber<>() {
+ private int lastVer = INITIAL_SCHEMA_VERSION;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ // Request unlimited demand.
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(Entry item) {
+ String key = new String(item.key(), StandardCharsets.UTF_8);
int descVer = extractVerFromSchemaKey(key);
if (descVer > lastVer) {
lastVer = descVer;
}
}
- return lastVer;
- } catch (NodeStoppingException e) {
- throw new IgniteException(e.traceId(), e.code(), e.getMessage(), e);
+ @Override
+ public void onError(Throwable throwable) {
+ latestVersionFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ latestVersionFuture.complete(lastVer);
+ }
+ });
+
+ try {
+ return latestVersionFuture.get(10, TimeUnit.SECONDS);
Review Comment:
Forget TODO
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1098329991
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaManager.java:
##########
@@ -432,22 +433,48 @@ public void stop() throws Exception {
* @param tblId Table id.
* @return The latest schema version.
*/
+ // TODO: Make this method async, see https://issues.apache.org/jira/browse/IGNITE-18732
private int latestSchemaVersion(UUID tblId) {
- try (Cursor<Entry> cur = metastorageMgr.prefix(schemaHistPrefix(tblId))) {
- int lastVer = INITIAL_SCHEMA_VERSION;
+ var latestVersionFuture = new CompletableFuture<Integer>();
- for (Entry ent : cur) {
- String key = new String(ent.key(), StandardCharsets.UTF_8);
+ metastorageMgr.prefix(schemaHistPrefix(tblId)).subscribe(new Subscriber<>() {
+ private int lastVer = INITIAL_SCHEMA_VERSION;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ // Request unlimited demand.
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(Entry item) {
+ String key = new String(item.key(), StandardCharsets.UTF_8);
int descVer = extractVerFromSchemaKey(key);
if (descVer > lastVer) {
lastVer = descVer;
}
}
- return lastVer;
- } catch (NodeStoppingException e) {
- throw new IgniteException(e.traceId(), e.code(), e.getMessage(), e);
+ @Override
+ public void onError(Throwable throwable) {
+ latestVersionFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ latestVersionFuture.complete(lastVer);
+ }
+ });
+
+ try {
+ return latestVersionFuture.get(10, TimeUnit.SECONDS);
Review Comment:
TODO is located on line 436
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099877581
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
ok, I'll add a protection just in case, it doesn't really make much sense to have multiple subscribers in this case
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1098378324
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageServiceImpl.java:
##########
@@ -82,29 +88,27 @@ public class MetaStorageServiceImpl implements MetaStorageService {
public MetaStorageServiceImpl(RaftGroupService metaStorageRaftGrpSvc, ClusterNode localNode) {
this.metaStorageRaftGrpSvc = metaStorageRaftGrpSvc;
this.localNode = localNode;
+ this.executorService = Executors.newFixedThreadPool(5, NamedThreadFactory.create(localNode.name(), "metastorage-publisher", LOG));
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099745911
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
+
+ WriteCommand createCursorCommand = createCursorSupplier.apply(cursorId);
+
+ raftService.run(createCursorCommand)
Review Comment:
Makes sense.
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099995151
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
+ responseIndex = 0;
+
+ processRequest();
+ } else {
+ onError(e);
+ }
+ }, executorService);
+ }
+
+ @Override
+ public void cancel() {
+ isDone = true;
Review Comment:
Shame on you, you are right.
`Subscription.request and Subscription.cancel MUST only be called inside of its Subscriber context.`
https://github.com/reactive-streams/reactive-streams-jvm/
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097334101
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
Review Comment:
Isn't a parallel collection needed here?
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(T item) {
+ items.add(item);
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.complete(items);
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Subscribes to the given publisher, extracting a single value from it.
+ */
+ public static <T> CompletableFuture<T> subscribeToValue(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<T>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private Subscription subscription;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ this.subscription = subscription;
+
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(T item) {
+ resultFuture.complete(item);
+
+ subscription.cancel();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.completeExceptionally(new NoSuchElementException());
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Creates a Publisher that provides data form a given cursor.
+ */
+ public static <T> Publisher<T> fromCursor(Cursor<T> cursor) {
+ return subscriber -> subscriber.onSubscribe(new Subscription() {
+ private long demand;
+
+ private boolean isStarted;
+
+ private boolean isCancelled;
+
+ @Override
+ public void request(long n) {
+ demand += n;
+
+ if (!isStarted) {
+ isStarted = true;
+
+ start();
+ }
+ }
+
+ private void start() {
+ ForkJoinPool.commonPool().execute(() -> {
Review Comment:
I think you need to use a separate pool.
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(T item) {
+ items.add(item);
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.complete(items);
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Subscribes to the given publisher, extracting a single value from it.
+ */
+ public static <T> CompletableFuture<T> subscribeToValue(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<T>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private Subscription subscription;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ this.subscription = subscription;
+
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(T item) {
+ resultFuture.complete(item);
+
+ subscription.cancel();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.completeExceptionally(new NoSuchElementException());
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Creates a Publisher that provides data form a given cursor.
+ */
+ public static <T> Publisher<T> fromCursor(Cursor<T> cursor) {
+ return subscriber -> subscriber.onSubscribe(new Subscription() {
+ private long demand;
+
+ private boolean isStarted;
+
+ private boolean isCancelled;
Review Comment:
Shouldn't these variables be thread-safe?
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
Review Comment:
Let's rename to `TestFlowUtils` to make it clearer that this is from the test framework.
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
Review Comment:
Shouldn't variables be thread-safe?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java:
##########
@@ -63,32 +63,40 @@ public interface MetaStorageManager extends IgniteComponent {
* {@code revUpperBound == -1}.
*
* @param keyPrefix Prefix of the key to retrieve the entries. Couldn't be {@code null}.
- * @return Cursor built upon entries corresponding to the given range and revision.
+ * @return Publisher that will provide entries corresponding to the given prefix.
* @throws OperationTimeoutException If the operation is timed out.
* @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
* @see ByteArray
* @see Entry
*/
- Cursor<Entry> prefix(ByteArray keyPrefix) throws NodeStoppingException;
+ Publisher<Entry> prefix(ByteArray keyPrefix);
/**
* Retrieves entries for the given key prefix in lexicographic order. Entries will be filtered out by upper bound of given revision
* number.
*
* @param keyPrefix Prefix of the key to retrieve the entries. Couldn't be {@code null}.
* @param revUpperBound The upper bound for entry revision. {@code -1} means latest revision.
- * @return Cursor built upon entries corresponding to the given range and revision.
+ * @return Publisher that will provide entries corresponding to the given prefix and revision.
* @throws OperationTimeoutException If the operation is timed out.
* @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
* @see ByteArray
* @see Entry
*/
- Cursor<Entry> prefix(ByteArray keyPrefix, long revUpperBound) throws NodeStoppingException;
+ Publisher<Entry> prefix(ByteArray keyPrefix, long revUpperBound);
/**
* Retrieves entries for the given key range in lexicographic order.
+ *
+ * @param keyFrom Range lower bound (inclusive).
+ * @param keyTo Range upper bound (exclusive), {@code null} represents an unbound range.
+ * @return Publisher that will provide entries corresponding to the given range.
+ * @throws OperationTimeoutException If the operation is timed out.
+ * @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
+ * @see ByteArray
+ * @see Entry
*/
- Cursor<Entry> range(ByteArray keyFrom, @Nullable ByteArray keyTo) throws NodeStoppingException;
+ Publisher<Entry> range(ByteArray keyFrom, @Nullable ByteArray keyTo);
Review Comment:
Same
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
Review Comment:
same
##########
modules/metastorage/src/test/java/org/apache/ignite/internal/metastorage/impl/CursorPublisherTest.java:
##########
@@ -0,0 +1,184 @@
+/*
+ * 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.metastorage.impl;
+
+import static java.util.Collections.nCopies;
+import static java.util.concurrent.CompletableFuture.completedFuture;
+import static java.util.concurrent.CompletableFuture.failedFuture;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.will;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willFailFast;
+import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willFailIn;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.TimeUnit;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.CreateRangeCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.internal.testframework.flow.FlowUtils;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+/**
+ * Tests for {@link CursorPublisher} and {@link CursorSubscription}.
+ */
+@ExtendWith(MockitoExtension.class)
+public class CursorPublisherTest {
+ @Mock
+ private RaftGroupService raftService;
+
+ private CursorPublisher publisher;
+
+ @BeforeEach
+ void setUp() {
+ publisher = new CursorPublisher(
+ raftService,
+ new MetaStorageCommandsFactory(),
+ ForkJoinPool.commonPool(),
Review Comment:
https://avatars.mds.yandex.net/i?id=1cbe54508db85747fa40c700b3a1d6fab3115558-7086141-images-thumbs&n=13
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaManager.java:
##########
@@ -433,21 +434,46 @@ public void stop() throws Exception {
* @return The latest schema version.
*/
private int latestSchemaVersion(UUID tblId) {
- try (Cursor<Entry> cur = metastorageMgr.prefix(schemaHistPrefix(tblId))) {
- int lastVer = INITIAL_SCHEMA_VERSION;
+ var latestVersionFuture = new CompletableFuture<Integer>();
- for (Entry ent : cur) {
- String key = new String(ent.key(), StandardCharsets.UTF_8);
+ metastorageMgr.prefix(schemaHistPrefix(tblId)).subscribe(new Subscriber<>() {
+ private int lastVer = INITIAL_SCHEMA_VERSION;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ // Request unlimited demand.
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(Entry item) {
+ String key = new String(item.key(), StandardCharsets.UTF_8);
int descVer = extractVerFromSchemaKey(key);
if (descVer > lastVer) {
lastVer = descVer;
}
}
- return lastVer;
- } catch (NodeStoppingException e) {
- throw new IgniteException(e.traceId(), e.code(), e.getMessage(), e);
+ @Override
+ public void onError(Throwable throwable) {
+ latestVersionFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ latestVersionFuture.complete(lastVer);
+ }
+ });
+
+ try {
+ return latestVersionFuture.get(10, TimeUnit.SECONDS);
Review Comment:
This is bad, you need to get rid of the synchronous code.
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java:
##########
@@ -63,32 +63,40 @@ public interface MetaStorageManager extends IgniteComponent {
* {@code revUpperBound == -1}.
*
* @param keyPrefix Prefix of the key to retrieve the entries. Couldn't be {@code null}.
- * @return Cursor built upon entries corresponding to the given range and revision.
+ * @return Publisher that will provide entries corresponding to the given prefix.
* @throws OperationTimeoutException If the operation is timed out.
* @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
* @see ByteArray
* @see Entry
*/
- Cursor<Entry> prefix(ByteArray keyPrefix) throws NodeStoppingException;
+ Publisher<Entry> prefix(ByteArray keyPrefix);
Review Comment:
Why not throw an **NodeStoppingException** if the node has stopped?
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java:
##########
@@ -63,32 +63,40 @@ public interface MetaStorageManager extends IgniteComponent {
* {@code revUpperBound == -1}.
*
* @param keyPrefix Prefix of the key to retrieve the entries. Couldn't be {@code null}.
- * @return Cursor built upon entries corresponding to the given range and revision.
+ * @return Publisher that will provide entries corresponding to the given prefix.
* @throws OperationTimeoutException If the operation is timed out.
* @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
* @see ByteArray
* @see Entry
*/
- Cursor<Entry> prefix(ByteArray keyPrefix) throws NodeStoppingException;
+ Publisher<Entry> prefix(ByteArray keyPrefix);
/**
* Retrieves entries for the given key prefix in lexicographic order. Entries will be filtered out by upper bound of given revision
* number.
*
* @param keyPrefix Prefix of the key to retrieve the entries. Couldn't be {@code null}.
* @param revUpperBound The upper bound for entry revision. {@code -1} means latest revision.
- * @return Cursor built upon entries corresponding to the given range and revision.
+ * @return Publisher that will provide entries corresponding to the given prefix and revision.
* @throws OperationTimeoutException If the operation is timed out.
* @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
* @see ByteArray
* @see Entry
*/
- Cursor<Entry> prefix(ByteArray keyPrefix, long revUpperBound) throws NodeStoppingException;
+ Publisher<Entry> prefix(ByteArray keyPrefix, long revUpperBound);
Review Comment:
Same
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(T item) {
+ items.add(item);
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.complete(items);
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Subscribes to the given publisher, extracting a single value from it.
+ */
+ public static <T> CompletableFuture<T> subscribeToValue(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<T>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private Subscription subscription;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ this.subscription = subscription;
+
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(T item) {
+ resultFuture.complete(item);
+
+ subscription.cancel();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.completeExceptionally(new NoSuchElementException());
Review Comment:
It should be described in the javaDoc.
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageServiceImpl.java:
##########
@@ -82,29 +88,27 @@ public class MetaStorageServiceImpl implements MetaStorageService {
public MetaStorageServiceImpl(RaftGroupService metaStorageRaftGrpSvc, ClusterNode localNode) {
this.metaStorageRaftGrpSvc = metaStorageRaftGrpSvc;
this.localNode = localNode;
+ this.executorService = Executors.newCachedThreadPool(NamedThreadFactory.create(localNode.name(), "metastorage-publisher", LOG));
Review Comment:
Could there be too many threads 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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099870134
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
+ responseIndex = 0;
+
+ processRequest();
+ } else {
+ onError(e);
+ }
+ }, executorService);
+ }
+
+ @Override
+ public void cancel() {
+ isDone = true;
Review Comment:
Can you explain how that could happen? I also expect this method to be called from the same thread (e.g. during `onNext` or `onError`).
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099735194
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
+
+ WriteCommand createCursorCommand = createCursorSupplier.apply(cursorId);
+
+ raftService.run(createCursorCommand)
+ .whenCompleteAsync((v, e) -> {
+ if (e == null) {
Review Comment:
Makes sense, I always forget about this stuff
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
I don't see any technical problems with that (we will simply create multiple cursors, one per subscription). Though I don't think it's useful in any way
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097370856
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
Review Comment:
what's the point? In the test package, you can't use it by accident
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097491175
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
Review Comment:
Fixed
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1098298706
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageServiceImpl.java:
##########
@@ -82,29 +88,27 @@ public class MetaStorageServiceImpl implements MetaStorageService {
public MetaStorageServiceImpl(RaftGroupService metaStorageRaftGrpSvc, ClusterNode localNode) {
this.metaStorageRaftGrpSvc = metaStorageRaftGrpSvc;
this.localNode = localNode;
+ this.executorService = Executors.newFixedThreadPool(5, NamedThreadFactory.create(localNode.name(), "metastorage-publisher", LOG));
Review Comment:
Maybe the number of CPUs?
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099733168
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
+
+ WriteCommand createCursorCommand = createCursorSupplier.apply(cursorId);
+
+ raftService.run(createCursorCommand)
Review Comment:
I will rework this part soon, when we are going to migrate to ReadCommands for cursors
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099743024
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
Review Comment:
> Is it always singleTreaded executor?
No
> Do you expect that whenCompleteAsync will be executed in the same thread as CursorSubscription.request(n)
No, but I expect that there's a following HB relationships:
1. For the first ever call of `request`: request -> submission on the executor. Though now I think that this may be debatable, because we are completing the future from a Raft service thread. Looking at the code I still think that there's a HB there because of the future chains, but maybe it makes sense to make `demand` variable volatile, what do you think?
2. For all consecutive calls `request` is expected to be done in the same thread as the `processRequest` (because it is probably called inside the `onNext` callback)
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099872779
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
Review Comment:
Correct, there should be no concurrent `requestNextBatch` at the same moment, though consecutive batches can be processed on different threads (and I expect that we have a HB through future chains there)
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097403190
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
Review Comment:
Discussed in private
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(T item) {
+ items.add(item);
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.complete(items);
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Subscribes to the given publisher, extracting a single value from it.
+ */
+ public static <T> CompletableFuture<T> subscribeToValue(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<T>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private Subscription subscription;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ this.subscription = subscription;
+
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(T item) {
+ resultFuture.complete(item);
+
+ subscription.cancel();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.completeExceptionally(new NoSuchElementException());
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Creates a Publisher that provides data form a given cursor.
+ */
+ public static <T> Publisher<T> fromCursor(Cursor<T> cursor) {
+ return subscriber -> subscriber.onSubscribe(new Subscription() {
+ private long demand;
+
+ private boolean isStarted;
+
+ private boolean isCancelled;
Review Comment:
Discussed in private
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099794001
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
Actually, I gave it a thought and maybe it breaks the Publisher contract a bit (that is, a Publisher being a single stream of data, while multiple cursors are independent). Do you think we should bother and protect against that or my understanding is incorrect?
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097371155
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(T item) {
+ items.add(item);
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.complete(items);
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Subscribes to the given publisher, extracting a single value from it.
+ */
+ public static <T> CompletableFuture<T> subscribeToValue(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<T>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private Subscription subscription;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ this.subscription = subscription;
+
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(T item) {
+ resultFuture.complete(item);
+
+ subscription.cancel();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.completeExceptionally(new NoSuchElementException());
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Creates a Publisher that provides data form a given cursor.
+ */
+ public static <T> Publisher<T> fromCursor(Cursor<T> cursor) {
+ return subscriber -> subscriber.onSubscribe(new Subscription() {
+ private long demand;
+
+ private boolean isStarted;
+
+ private boolean isCancelled;
Review Comment:
nope
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097370499
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
Review Comment:
no, why?
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097403632
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
Review Comment:
Discussed in private
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
Review Comment:
Discussed in private
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1098326147
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageServiceImpl.java:
##########
@@ -82,29 +88,27 @@ public class MetaStorageServiceImpl implements MetaStorageService {
public MetaStorageServiceImpl(RaftGroupService metaStorageRaftGrpSvc, ClusterNode localNode) {
this.metaStorageRaftGrpSvc = metaStorageRaftGrpSvc;
this.localNode = localNode;
+ this.executorService = Executors.newFixedThreadPool(5, NamedThreadFactory.create(localNode.name(), "metastorage-publisher", LOG));
Review Comment:
Will there be any problems if the user only has 4 and 32 CPUs? Will there be starvation or underutilization of resources?
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097402437
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaManager.java:
##########
@@ -433,21 +434,46 @@ public void stop() throws Exception {
* @return The latest schema version.
*/
private int latestSchemaVersion(UUID tblId) {
- try (Cursor<Entry> cur = metastorageMgr.prefix(schemaHistPrefix(tblId))) {
- int lastVer = INITIAL_SCHEMA_VERSION;
+ var latestVersionFuture = new CompletableFuture<Integer>();
- for (Entry ent : cur) {
- String key = new String(ent.key(), StandardCharsets.UTF_8);
+ metastorageMgr.prefix(schemaHistPrefix(tblId)).subscribe(new Subscriber<>() {
+ private int lastVer = INITIAL_SCHEMA_VERSION;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ // Request unlimited demand.
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(Entry item) {
+ String key = new String(item.key(), StandardCharsets.UTF_8);
int descVer = extractVerFromSchemaKey(key);
if (descVer > lastVer) {
lastVer = descVer;
}
}
- return lastVer;
- } catch (NodeStoppingException e) {
- throw new IgniteException(e.traceId(), e.code(), e.getMessage(), e);
+ @Override
+ public void onError(Throwable throwable) {
+ latestVersionFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ latestVersionFuture.complete(lastVer);
+ }
+ });
+
+ try {
+ return latestVersionFuture.get(10, TimeUnit.SECONDS);
Review Comment:
Please add TODO with a ticket.
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1098343773
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageServiceImpl.java:
##########
@@ -82,29 +88,27 @@ public class MetaStorageServiceImpl implements MetaStorageService {
public MetaStorageServiceImpl(RaftGroupService metaStorageRaftGrpSvc, ClusterNode localNode) {
this.metaStorageRaftGrpSvc = metaStorageRaftGrpSvc;
this.localNode = localNode;
+ this.executorService = Executors.newFixedThreadPool(5, NamedThreadFactory.create(localNode.name(), "metastorage-publisher", LOG));
Review Comment:
No, this thread pool is not CPU bound, it's tied to the amount of requests to the Meta Storage. 5 is an arbitrary value, I will add a TODO to extract this value to configuration.
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099745911
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
+
+ WriteCommand createCursorCommand = createCursorSupplier.apply(cursorId);
+
+ raftService.run(createCursorCommand)
Review Comment:
Make sense.
--
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
[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097404864
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java:
##########
@@ -63,32 +63,40 @@ public interface MetaStorageManager extends IgniteComponent {
* {@code revUpperBound == -1}.
*
* @param keyPrefix Prefix of the key to retrieve the entries. Couldn't be {@code null}.
- * @return Cursor built upon entries corresponding to the given range and revision.
+ * @return Publisher that will provide entries corresponding to the given prefix.
* @throws OperationTimeoutException If the operation is timed out.
* @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
* @see ByteArray
* @see Entry
*/
- Cursor<Entry> prefix(ByteArray keyPrefix) throws NodeStoppingException;
+ Publisher<Entry> prefix(ByteArray keyPrefix);
Review Comment:
Please describe in the javaDoc.
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099732732
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
I don't see any technical problems with that (we will simply create multiple cursors, one per subscription). Thought I don't think it's useful in any way
--
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
[GitHub] [ignite-3] sanpwc merged pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc merged PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099372784
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
It's covered with outer busyLock, correct?
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
+
+ WriteCommand createCursorCommand = createCursorSupplier.apply(cursorId);
+
+ raftService.run(createCursorCommand)
Review Comment:
Did you consider the solution that will eliminate additional rtt on cursor creation? E.g. it's possible return BatchResponse even for cursor creation and postpone cursor creation until explicit request within onSubscribe.
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
Review Comment:
Not sure that I get it. Do you expect that whenCompleteAsync will be executed in the same thread as CursorSubscription.request(n)?
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
So, you do support multiple subscriptions, right?
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
+
+ WriteCommand createCursorCommand = createCursorSupplier.apply(cursorId);
+
+ raftService.run(createCursorCommand)
+ .whenCompleteAsync((v, e) -> {
+ if (e == null) {
Review Comment:
Should we also cover such code with busyLock?
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099862651
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
Review Comment:
My fault, I've missed the
```
if (cachedResponse == null) {
requestNextBatch();
}
```
part. So you'll never request next portion before current one is fully processed, so there will be no multiple requestNextBatch calls on different threads.
Seems that demand should be fine without being volatile.
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099933287
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
+ responseIndex = 0;
+
+ processRequest();
+ } else {
+ onError(e);
+ }
+ }, executorService);
+ }
+
+ @Override
+ public void cancel() {
+ isDone = true;
Review Comment:
I mean following (pseudocode)
```
private volatile Subscription msRangeSub;
ms.range().subscribe(new Subscriber<Object>() {
@Override
public void onSubscribe (Subscription subscription){
this.msRangeSub = subscription;
}
}
stop() {
msRangeSub.cancel();
}
```
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097371792
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(T item) {
+ items.add(item);
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.complete(items);
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Subscribes to the given publisher, extracting a single value from it.
+ */
+ public static <T> CompletableFuture<T> subscribeToValue(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<T>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private Subscription subscription;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ this.subscription = subscription;
+
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(T item) {
+ resultFuture.complete(item);
+
+ subscription.cancel();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.completeExceptionally(new NoSuchElementException());
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Creates a Publisher that provides data form a given cursor.
+ */
+ public static <T> Publisher<T> fromCursor(Cursor<T> cursor) {
+ return subscriber -> subscriber.onSubscribe(new Subscription() {
+ private long demand;
+
+ private boolean isStarted;
+
+ private boolean isCancelled;
+
+ @Override
+ public void request(long n) {
+ demand += n;
+
+ if (!isStarted) {
+ isStarted = true;
+
+ start();
+ }
+ }
+
+ private void start() {
+ ForkJoinPool.commonPool().execute(() -> {
Review Comment:
why? This is test-only class. And a separate pool requires starting and stopping it, which is not convenient
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097377262
##########
modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaManager.java:
##########
@@ -433,21 +434,46 @@ public void stop() throws Exception {
* @return The latest schema version.
*/
private int latestSchemaVersion(UUID tblId) {
- try (Cursor<Entry> cur = metastorageMgr.prefix(schemaHistPrefix(tblId))) {
- int lastVer = INITIAL_SCHEMA_VERSION;
+ var latestVersionFuture = new CompletableFuture<Integer>();
- for (Entry ent : cur) {
- String key = new String(ent.key(), StandardCharsets.UTF_8);
+ metastorageMgr.prefix(schemaHistPrefix(tblId)).subscribe(new Subscriber<>() {
+ private int lastVer = INITIAL_SCHEMA_VERSION;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ // Request unlimited demand.
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(Entry item) {
+ String key = new String(item.key(), StandardCharsets.UTF_8);
int descVer = extractVerFromSchemaKey(key);
if (descVer > lastVer) {
lastVer = descVer;
}
}
- return lastVer;
- } catch (NodeStoppingException e) {
- throw new IgniteException(e.traceId(), e.code(), e.getMessage(), e);
+ @Override
+ public void onError(Throwable throwable) {
+ latestVersionFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ latestVersionFuture.complete(lastVer);
+ }
+ });
+
+ try {
+ return latestVersionFuture.get(10, TimeUnit.SECONDS);
Review Comment:
I can't do it in this PR, that's too many changes. And this code was synchronous before
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097376120
##########
modules/metastorage-api/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java:
##########
@@ -63,32 +63,40 @@ public interface MetaStorageManager extends IgniteComponent {
* {@code revUpperBound == -1}.
*
* @param keyPrefix Prefix of the key to retrieve the entries. Couldn't be {@code null}.
- * @return Cursor built upon entries corresponding to the given range and revision.
+ * @return Publisher that will provide entries corresponding to the given prefix.
* @throws OperationTimeoutException If the operation is timed out.
* @throws CompactedException If the desired revisions are removed from the storage due to a compaction.
* @see ByteArray
* @see Entry
*/
- Cursor<Entry> prefix(ByteArray keyPrefix) throws NodeStoppingException;
+ Publisher<Entry> prefix(ByteArray keyPrefix);
Review Comment:
Because this is an async API. All other async methods (that return `CompletableFuture`), do not throw these exceptions
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097376411
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageServiceImpl.java:
##########
@@ -82,29 +88,27 @@ public class MetaStorageServiceImpl implements MetaStorageService {
public MetaStorageServiceImpl(RaftGroupService metaStorageRaftGrpSvc, ClusterNode localNode) {
this.metaStorageRaftGrpSvc = metaStorageRaftGrpSvc;
this.localNode = localNode;
+ this.executorService = Executors.newCachedThreadPool(NamedThreadFactory.create(localNode.name(), "metastorage-publisher", LOG));
Review Comment:
I don't know(
--
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
[GitHub] [ignite-3] SammyVimes commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "SammyVimes (via GitHub)" <gi...@apache.org>.
SammyVimes commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1097405723
##########
modules/core/src/testFixtures/java/org/apache/ignite/internal/testframework/flow/FlowUtils.java:
##########
@@ -0,0 +1,158 @@
+/*
+ * 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.testframework.flow;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.NoSuchElementException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import java.util.concurrent.ForkJoinPool;
+import org.apache.ignite.internal.util.Cursor;
+
+/**
+ * Utility methods for extracting data from {@link Publisher}s.
+ */
+public class FlowUtils {
+ /**
+ * Subscribes to the given publisher, collecting all values into a list.
+ */
+ public static <T> CompletableFuture<List<T>> subscribeToList(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<List<T>>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private final List<T> items = new ArrayList<>();
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ subscription.request(Long.MAX_VALUE);
+ }
+
+ @Override
+ public void onNext(T item) {
+ items.add(item);
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.complete(items);
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Subscribes to the given publisher, extracting a single value from it.
+ */
+ public static <T> CompletableFuture<T> subscribeToValue(Publisher<T> publisher) {
+ var resultFuture = new CompletableFuture<T>();
+
+ publisher.subscribe(new Subscriber<>() {
+ private Subscription subscription;
+
+ @Override
+ public void onSubscribe(Subscription subscription) {
+ this.subscription = subscription;
+
+ subscription.request(1);
+ }
+
+ @Override
+ public void onNext(T item) {
+ resultFuture.complete(item);
+
+ subscription.cancel();
+ }
+
+ @Override
+ public void onError(Throwable throwable) {
+ resultFuture.completeExceptionally(throwable);
+ }
+
+ @Override
+ public void onComplete() {
+ resultFuture.completeExceptionally(new NoSuchElementException());
+ }
+ });
+
+ return resultFuture;
+ }
+
+ /**
+ * Creates a Publisher that provides data form a given cursor.
Review Comment:
```suggestion
* Creates a Publisher that provides data from a given cursor.
```
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099876236
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
Well, I believe that you should either prevent multiple subscriptions or notify all registered ones on every new entry, error, etc. Currently you subscriptions are completely independent, so that each behaves as isolated PubSub pair instead of Pub(Sub1 + Sub2 + ...).
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099400275
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
Review Comment:
Not sure that I get it. Do you expect that whenCompleteAsync will be executed in the same thread as CursorSubscription.request(n)? Is it always singleTreaded executor?
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099372784
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorPublisher.java:
##########
@@ -0,0 +1,85 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.UUID;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Publisher;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.function.Function;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.raft.WriteCommand;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.apache.ignite.lang.IgniteUuidGenerator;
+
+/**
+ * Publisher that streams data from a remote Meta Storage cursor.
+ *
+ * @see CursorSubscription
+ */
+class CursorPublisher implements Publisher<Entry> {
+ private static final IgniteUuidGenerator UUID_GENERATOR = new IgniteUuidGenerator(UUID.randomUUID(), 0);
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final Function<IgniteUuid, WriteCommand> createCursorSupplier;
+
+ /**
+ * Creates a new publisher instance.
+ *
+ * @param raftService Meta Storage Raft service.
+ * @param commandsFactory Meta Storage commands factory.
+ * @param executorService Executor that will be used to call the subscriber's methods.
+ * @param createCursorSupplier Factory that creates a remote Meta Storage cursor (provided with a cursor ID).
+ */
+ CursorPublisher(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ Function<IgniteUuid, WriteCommand> createCursorSupplier
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.createCursorSupplier = createCursorSupplier;
+ }
+
+ @Override
+ public void subscribe(Subscriber<? super Entry> subscriber) {
+ IgniteUuid cursorId = UUID_GENERATOR.randomUuid();
Review Comment:
It's covered with outer busyLock, correct?
--
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
[GitHub] [ignite-3] sanpwc commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1099865791
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/CursorSubscription.java:
##########
@@ -0,0 +1,185 @@
+/*
+ * 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.metastorage.impl;
+
+import java.util.List;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Flow.Subscriber;
+import java.util.concurrent.Flow.Subscription;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+import org.apache.ignite.internal.metastorage.Entry;
+import org.apache.ignite.internal.metastorage.command.MetaStorageCommandsFactory;
+import org.apache.ignite.internal.metastorage.command.cursor.CloseCursorCommand;
+import org.apache.ignite.internal.metastorage.command.cursor.NextBatchCommand;
+import org.apache.ignite.internal.metastorage.command.response.BatchResponse;
+import org.apache.ignite.internal.raft.service.RaftGroupService;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Subscription used to notify a subscriber on Meta Storage cursor data.
+ *
+ * @implNote Subscription is always processed sequentially in one of the provided executor's thread, so no explicit synchronization is used.
+ * It is expected that subscription methods (i.e. {@code request} and {@code cancel}) are called from the same thread that
+ * was used to invoke {@code onNext}.
+ */
+class CursorSubscription implements Subscription {
+ private static final IgniteLogger LOG = Loggers.forClass(MetaStorageService.class);
+
+ /** Default batch size that is requested from the remote server. */
+ private static final int BATCH_SIZE = 1000;
+
+ private final RaftGroupService raftService;
+
+ private final MetaStorageCommandsFactory commandsFactory;
+
+ private final ExecutorService executorService;
+
+ private final IgniteUuid cursorId;
+
+ private final Subscriber<? super Entry> subscriber;
+
+ /** Flag indicating that either the whole data range has been exhausted or the subscription has been cancelled. */
+ private boolean isDone = false;
+
+ /** Cached response from the Meta Storage leader. {@code null} if no requests have been issued yet. */
+ @Nullable
+ private BatchResponse cachedResponse;
+
+ /** Index inside the cached response. */
+ private int responseIndex;
+
+ /** Amount of entries requested by the subscriber. */
+ private long demand;
+
+ CursorSubscription(
+ RaftGroupService raftService,
+ MetaStorageCommandsFactory commandsFactory,
+ ExecutorService executorService,
+ IgniteUuid cursorId,
+ Subscriber<? super Entry> subscriber
+ ) {
+ this.raftService = raftService;
+ this.commandsFactory = commandsFactory;
+ this.executorService = executorService;
+ this.cursorId = cursorId;
+ this.subscriber = subscriber;
+ }
+
+ @Override
+ public void request(long n) {
+ if (n <= 0) {
+ onError(new IllegalArgumentException("Requested amount must be greater than zero, got: " + n));
+
+ return;
+ }
+
+ if (isDone) {
+ return;
+ }
+
+ // Increase the number of requested items.
+ demand += n;
+
+ if (demand <= 0) {
+ onError(new IllegalArgumentException("Long overflow"));
+
+ return;
+ }
+
+ // Start the processing if it has not been started yet (async operation).
+ if (cachedResponse == null) {
+ requestNextBatch();
+ }
+ }
+
+ /**
+ * Does the actual request processing. Must always be called on {@link CursorSubscription#executorService}.
+ */
+ private void processRequest() {
+ if (isDone) {
+ return;
+ }
+
+ try {
+ assert cachedResponse != null;
+
+ List<Entry> entries = cachedResponse.entries();
+
+ while (demand > 0 && !isDone) {
+ if (responseIndex < entries.size()) {
+ subscriber.onNext(entries.get(responseIndex));
+
+ responseIndex++;
+ demand--;
+ } else {
+ if (cachedResponse.hasNextBatch()) {
+ requestNextBatch();
+ } else {
+ isDone = true;
+
+ subscriber.onComplete();
+ }
+
+ return;
+ }
+ }
+ } catch (Exception e) {
+ onError(e);
+ }
+ }
+
+ private void requestNextBatch() {
+ NextBatchCommand command = commandsFactory.nextBatchCommand()
+ .cursorId(cursorId)
+ .batchSize(BATCH_SIZE)
+ .build();
+
+ raftService.<BatchResponse>run(command)
+ .whenCompleteAsync((resp, e) -> {
+ if (e == null) {
+ cachedResponse = resp;
+ responseIndex = 0;
+
+ processRequest();
+ } else {
+ onError(e);
+ }
+ }, executorService);
+ }
+
+ @Override
+ public void cancel() {
+ isDone = true;
Review Comment:
And what about this one? Seems that it's possible and valid to cancel subscriptions within some other threads, e.g. components.stop one.
--
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
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1626: IGNITE-14691 Implement reactive Meta Storage range methods
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #1626:
URL: https://github.com/apache/ignite-3/pull/1626#discussion_r1098315485
##########
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/impl/MetaStorageServiceImpl.java:
##########
@@ -82,29 +88,27 @@ public class MetaStorageServiceImpl implements MetaStorageService {
public MetaStorageServiceImpl(RaftGroupService metaStorageRaftGrpSvc, ClusterNode localNode) {
this.metaStorageRaftGrpSvc = metaStorageRaftGrpSvc;
this.localNode = localNode;
+ this.executorService = Executors.newFixedThreadPool(5, NamedThreadFactory.create(localNode.name(), "metastorage-publisher", LOG));
Review Comment:
why, how is this pool related to the number of CPUs? It's IO bound
--
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