You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/07/16 13:10:07 UTC
[GitHub] [ignite-3] sanpwc opened a new pull request #221: IGNITE-15133 Cursors and watches cleanup logic on node left.
sanpwc opened a new pull request #221:
URL: https://github.com/apache/ignite-3/pull/221
--
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] vldpyatkov commented on a change in pull request #221: IGNITE-15133 Cursors and watches cleanup logic on node left.
Posted by GitBox <gi...@apache.org>.
vldpyatkov commented on a change in pull request #221:
URL: https://github.com/apache/ignite-3/pull/221#discussion_r671501229
##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
##########
@@ -74,7 +76,7 @@
private final KeyValueStorage storage;
/** Cursors map. */
- private final Map<IgniteUuid, IgniteBiTuple<Cursor<?>, CursorType>> cursors;
+ private final Map<IgniteUuid, IgniteBiTuple<Cursor<?>, CursorMeta>> cursors;
Review comment:
It looks complicated. If I were you, I would add Cursor to CursorMeta.
##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
##########
@@ -303,10 +321,38 @@ else if (clo.command() instanceof WatchExactKeysCommand) {
Cursor<WatchEvent> cursor = storage.watch(watchCmd.keys(), watchCmd.revision());
- cursors.put(cursorId, new IgniteBiTuple<>(cursor, CursorType.WATCH));
+ cursors.put(
+ cursorId,
+ new IgniteBiTuple<>(
+ cursor,
+ new CursorMeta(CursorType.WATCH, watchCmd.requesterNodeId())
+ )
+ );
clo.result(cursorId);
}
+ else if (clo.command() instanceof CursorsCloseCommand) {
+ CursorsCloseCommand cursorsCloseCmd = (CursorsCloseCommand) clo.command();
+
+ for (Iterator<IgniteBiTuple<Cursor<?>, CursorMeta>> cursorsIter = cursors.values().iterator();
Review comment:
It looks ugly. Better to use 'while' here.
##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
##########
@@ -230,23 +232,33 @@ else if (clo.command() instanceof RangeCommand) {
rangeCmd.keyFrom(),
rangeCmd.keyTo());
- cursors.put(cursorId, new IgniteBiTuple<>(cursor, CursorType.RANGE));
+ cursors.put(
+ cursorId,
+ new IgniteBiTuple<>(
+ cursor,
+ new CursorMeta(CursorType.RANGE, rangeCmd.requesterNodeId())
+ )
+ );
clo.result(cursorId);
}
else if (clo.command() instanceof CursorNextCommand) {
CursorNextCommand cursorNextCmd = (CursorNextCommand) clo.command();
- assert cursors.containsKey(cursorNextCmd.cursorId());
+ IgniteBiTuple<Cursor<?>, CursorMeta> cursorDesc = cursors.get(cursorNextCmd.cursorId());
+
+ if (cursorDesc == null) {
Review comment:
Why did the cursor can not be found?
I think, need to add an extra information (the node left the cluster), also assertion for that.
--
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] asfgit closed pull request #221: IGNITE-15133 Cursors and watches cleanup logic on node left.
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #221:
URL: https://github.com/apache/ignite-3/pull/221
--
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 change in pull request #221: IGNITE-15133 Cursors and watches cleanup logic on node left.
Posted by GitBox <gi...@apache.org>.
sanpwc commented on a change in pull request #221:
URL: https://github.com/apache/ignite-3/pull/221#discussion_r672047404
##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
##########
@@ -74,7 +76,7 @@
private final KeyValueStorage storage;
/** Cursors map. */
- private final Map<IgniteUuid, IgniteBiTuple<Cursor<?>, CursorType>> cursors;
+ private final Map<IgniteUuid, IgniteBiTuple<Cursor<?>, CursorMeta>> cursors;
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] sanpwc commented on a change in pull request #221: IGNITE-15133 Cursors and watches cleanup logic on node left.
Posted by GitBox <gi...@apache.org>.
sanpwc commented on a change in pull request #221:
URL: https://github.com/apache/ignite-3/pull/221#discussion_r672054412
##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
##########
@@ -230,23 +232,33 @@ else if (clo.command() instanceof RangeCommand) {
rangeCmd.keyFrom(),
rangeCmd.keyTo());
- cursors.put(cursorId, new IgniteBiTuple<>(cursor, CursorType.RANGE));
+ cursors.put(
+ cursorId,
+ new IgniteBiTuple<>(
+ cursor,
+ new CursorMeta(CursorType.RANGE, rangeCmd.requesterNodeId())
+ )
+ );
clo.result(cursorId);
}
else if (clo.command() instanceof CursorNextCommand) {
CursorNextCommand cursorNextCmd = (CursorNextCommand) clo.command();
- assert cursors.containsKey(cursorNextCmd.cursorId());
+ IgniteBiTuple<Cursor<?>, CursorMeta> cursorDesc = cursors.get(cursorNextCmd.cursorId());
+
+ if (cursorDesc == null) {
Review comment:
Following comment was added:
```
// In real life we should never get here, cause it's only possible to get null if we try to retrieve
// next element for a cursor of a node that has already left topology.
// However testing with such an approach is much easier and we still respect the cursor's contract.
```
--
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 change in pull request #221: IGNITE-15133 Cursors and watches cleanup logic on node left.
Posted by GitBox <gi...@apache.org>.
sanpwc commented on a change in pull request #221:
URL: https://github.com/apache/ignite-3/pull/221#discussion_r672043130
##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
##########
@@ -230,23 +232,33 @@ else if (clo.command() instanceof RangeCommand) {
rangeCmd.keyFrom(),
rangeCmd.keyTo());
- cursors.put(cursorId, new IgniteBiTuple<>(cursor, CursorType.RANGE));
+ cursors.put(
+ cursorId,
+ new IgniteBiTuple<>(
+ cursor,
+ new CursorMeta(CursorType.RANGE, rangeCmd.requesterNodeId())
+ )
+ );
clo.result(cursorId);
}
else if (clo.command() instanceof CursorNextCommand) {
CursorNextCommand cursorNextCmd = (CursorNextCommand) clo.command();
- assert cursors.containsKey(cursorNextCmd.cursorId());
+ IgniteBiTuple<Cursor<?>, CursorMeta> cursorDesc = cursors.get(cursorNextCmd.cursorId());
+
+ if (cursorDesc == null) {
Review comment:
In real life you should never get here, cause it's only possible to get null if we try to retrieve next element for a node that have already left topology. However it's ease testing dramatically and we still respect the cursor contract.
--
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 change in pull request #221: IGNITE-15133 Cursors and watches cleanup logic on node left.
Posted by GitBox <gi...@apache.org>.
sanpwc commented on a change in pull request #221:
URL: https://github.com/apache/ignite-3/pull/221#discussion_r672038941
##########
File path: modules/metastorage-server/src/main/java/org/apache/ignite/internal/metastorage/server/raft/MetaStorageListener.java
##########
@@ -303,10 +321,38 @@ else if (clo.command() instanceof WatchExactKeysCommand) {
Cursor<WatchEvent> cursor = storage.watch(watchCmd.keys(), watchCmd.revision());
- cursors.put(cursorId, new IgniteBiTuple<>(cursor, CursorType.WATCH));
+ cursors.put(
+ cursorId,
+ new IgniteBiTuple<>(
+ cursor,
+ new CursorMeta(CursorType.WATCH, watchCmd.requesterNodeId())
+ )
+ );
clo.result(cursorId);
}
+ else if (clo.command() instanceof CursorsCloseCommand) {
+ CursorsCloseCommand cursorsCloseCmd = (CursorsCloseCommand) clo.command();
+
+ for (Iterator<IgniteBiTuple<Cursor<?>, CursorMeta>> cursorsIter = cursors.values().iterator();
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