You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "tkalkirill (via GitHub)" <gi...@apache.org> on 2023/01/30 13:59:47 UTC
[GitHub] [ignite-3] tkalkirill opened a new pull request, #1599: IGNITE-17958 PartitionListener should not close partition storage
tkalkirill opened a new pull request, #1599:
URL: https://github.com/apache/ignite-3/pull/1599
https://issues.apache.org/jira/browse/IGNITE-17958
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1092024540
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -292,6 +295,9 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
private static final TableMessagesFactory TABLE_MESSAGES_FACTORY = new TableMessagesFactory();
+ /** Futures for destroying table partition storages({@link MvPartitionStorage} and {@link TxStateStorage}). */
+ private static final Map<TablePartitionId, CompletableFuture<Void>> destroyFutureByTablePartitionId = new ConcurrentHashMap<>();
Review Comment:
Typo. fix it =)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091874046
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
The problem of clearing the storages when the node is restarted will be solved [IGNITE-18160](https://issues.apache.org/jira/browse/IGNITE-18160) when the node is stopped, we do not need to wait for the data to be cleared, it can take a long time.
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091735042
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
Methods `TxStateTableStorage#destroyTxStateStorage` and `MvTableStorage#destroyPartition` close the storages and clean up the data.
Thus, if we close the storages, then in order to clear them, we will need to create them again, which may not look good.
But in fact, deleting storages is done in almost O(1) at the moment.
I can describe it in the comments so that there are fewer questions.
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091920169
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
And what about storage recreation?
What I want to say is that if we want to drop the storage with some id X we can put destroyMvPartitionStorageFuture to the map using X as a key, and if we try to recreate the storage with same id we can chain storage creation with storage drop.
> we do not need to wait for the data to be cleared, it can take a long time.
That's a bit confusing. You've told that it's O(1)
> But in fact, deleting storages is done in almost O(1)
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091947811
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
Discussed personally, decided to do it through an external map with destruction futures.
We will wait for them at the re-creation of storages.
As well as at stopping nodes.
--
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 merged pull request #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill merged PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091735042
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
Methods `TxStateTableStorage#destroyTxStateStorage` and `MvTableStorage#destroyPartition` close the storages and clean up the data.
Thus, if we close the storages, then in order to clear them, we will need to create them again, which may not look good.
But in fact, deleting storages is done in almost O(1) at the moment, we just close storages and delete files for persistent storages, and for volatile storages we put tasks for cleaning in separate pools.
I can describe it in the comments so that there are fewer questions.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
Methods `TxStateTableStorage#destroyTxStateStorage` and `MvTableStorage#destroyPartition` close the storages and clean up the data.
Thus, if we close the storages, then in order to clear them, we will need to create them again, which may not look good.
But in fact, deleting storages is done in almost O(1) at the moment, we just close storages and delete files for persistent storages, and for volatile storages we put tasks for cleaning in separate pools.
I can describe it in the comments so that there are fewer questions.
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091762476
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
Review Comment:
As far as I understand, in the general case, deletion of storages will occur here, since deletion: closing + clearing data, then why not delete them now.
In [IGNITE-18160](https://issues.apache.org/jira/browse/IGNITE-18160,) we also need to add a mechanism to clean up storages when nodes are restarted.
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1092018538
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -292,6 +295,9 @@ public class TableManager extends Producer<TableEvent, TableEventParameters> imp
private static final TableMessagesFactory TABLE_MESSAGES_FACTORY = new TableMessagesFactory();
+ /** Futures for destroying table partition storages({@link MvPartitionStorage} and {@link TxStateStorage}). */
+ private static final Map<TablePartitionId, CompletableFuture<Void>> destroyFutureByTablePartitionId = new ConcurrentHashMap<>();
Review Comment:
Why it's static?
--
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] rpuch commented on a diff in pull request #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091736012
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
Review Comment:
I wonder why destruction is made under this Jira ticket: it's about closing the storage, not destruction. I thought we had another ticket about destruction of data of evicted partitions.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java:
##########
@@ -416,12 +416,6 @@ public boolean onSnapshotLoad(Path path) {
@Override
public void onShutdown() {
- // TODO: IGNITE-17958 - probably, we should not close the storage here as PartitionListener did not create the storage.
- try {
- storage.close();
Review Comment:
So the storage will be shut down either when the `TableStorage` is shutdown (on node stop), or when the partition is evicted from the node (in which case it will be not just shut down, but also destroyed)?
It would be great to have a test making sure that a non-evicted partition storage is closed on node stop.
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091781638
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/raft/PartitionListener.java:
##########
@@ -416,12 +416,6 @@ public boolean onSnapshotLoad(Path path) {
@Override
public void onShutdown() {
- // TODO: IGNITE-17958 - probably, we should not close the storage here as PartitionListener did not create the storage.
- try {
- storage.close();
Review Comment:
Storages will be closed when the `TableStorage` stops.
And are destroyed, on evict of partition or destruction of the table.
Found tests:
- org.apache.ignite.internal.table.distributed.TableManagerTest#endTableManagerStopTest
- org.apache.ignite.internal.table.distributed.TableManagerTest#testDropTable
- I don’t even know if testing is needed to evict a partition, it will be possible to add it during implementation [IGNITE-18160](https://issues.apache.org/jira/browse/IGNITE-18160,)
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091662368
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
Are you sure that it's necessary?
Why do we need to await the destruction?
If it's only about awaiting on component stop, it's possible to await allOf storage destruction futures before components stop itself.
--
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 #1599: IGNITE-17958 PartitionListener should not close partition storage
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #1599:
URL: https://github.com/apache/ignite-3/pull/1599#discussion_r1091829255
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1952,50 +1952,56 @@ public void onError(Throwable e) {
metaStorageMgr.registerPrefixWatch(ByteArray.fromString(STABLE_ASSIGNMENTS_PREFIX), new WatchListener() {
@Override
public void onUpdate(WatchEvent evt) {
- if (!busyLock.enterBusy()) {
- throw new IgniteInternalException(new NodeStoppingException());
- }
-
- try {
- assert evt.single();
+ inBusyLock(busyLock, () -> {
+ assert evt.single() : evt;
Entry stableAssignmentsWatchEvent = evt.entryEvent().newEntry();
if (stableAssignmentsWatchEvent.value() == null) {
return;
}
- int part = extractPartitionNumber(stableAssignmentsWatchEvent.key());
- UUID tblId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
+ int partitionId = extractPartitionNumber(stableAssignmentsWatchEvent.key());
+ UUID tableId = extractTableId(stableAssignmentsWatchEvent.key(), STABLE_ASSIGNMENTS_PREFIX);
- TablePartitionId replicaGrpId = new TablePartitionId(tblId, part);
+ TablePartitionId replicaGrpId = new TablePartitionId(tableId, partitionId);
Set<Assignment> stableAssignments = ByteUtils.fromBytes(stableAssignmentsWatchEvent.value());
- byte[] pendingFromMetastorage = metaStorageMgr.get(pendingPartAssignmentsKey(replicaGrpId),
- stableAssignmentsWatchEvent.revision()).join().value();
+ byte[] pendingAssignmentsFromMetaStorage = metaStorageMgr.get(
+ pendingPartAssignmentsKey(replicaGrpId),
+ stableAssignmentsWatchEvent.revision()
+ ).join().value();
- Set<Assignment> pendingAssignments = pendingFromMetastorage == null
+ Set<Assignment> pendingAssignments = pendingAssignmentsFromMetaStorage == null
? Set.of()
- : ByteUtils.fromBytes(pendingFromMetastorage);
+ : ByteUtils.fromBytes(pendingAssignmentsFromMetaStorage);
String localMemberName = clusterService.topologyService().localMember().name();
boolean shouldStopLocalServices = Stream.concat(stableAssignments.stream(), pendingAssignments.stream())
.noneMatch(assignment -> assignment.consistentId().equals(localMemberName));
if (shouldStopLocalServices) {
+ TableImpl table = tablesByIdVv.latest().get(tableId);
+
+ CompletableFuture<Void> destroyMvPartitionStorageFuture
+ = table.internalTable().storage().destroyPartition(partitionId);
+
+ table.internalTable().txStateStorage().destroyTxStateStorage(partitionId);
+
+ // Necessary evil.
Review Comment:
> Thus, if we close the storages, then in order to clear them, we will need to create them again, which may not look good.
My point is that in case of storage recreation we should chain it with storage.drop().
And besides that we should await storage drop on components stop.
--
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