You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "denis-chudov (via GitHub)" <gi...@apache.org> on 2025/05/27 16:46:27 UTC
[PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
denis-chudov opened a new pull request, #5915:
URL: https://github.com/apache/ignite-3/pull/5915
https://issues.apache.org/jira/browse/IGNITE-25421
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2140012257
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -446,7 +449,13 @@ private void handleReplicaRequest(ReplicaRequest request, ClusterNode sender, @N
private static boolean indicatesUnexpectedProblem(Throwable ex) {
Throwable unwrapped = unwrapCause(ex);
return !(unwrapped instanceof ExpectedReplicationException)
- && !hasCause(ex, NodeStoppingException.class, TrackerClosedException.class, ComponentStoppingException.class);
+ && !hasCause(
+ ex,
+ NodeStoppingException.class,
Review Comment:
can't we use ReplicatorRecoverableExceptions here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2140007826
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/WriteIntentSwitchProcessor.java:
##########
@@ -92,8 +92,9 @@ public CompletableFuture<WriteIntentSwitchReplicatedInfo> switchWriteIntentsWith
if (ex != null) {
Throwable cause = ExceptionUtils.unwrapCause(ex);
- if (TransactionFailureHandler.isRecoverable(cause)) {
- LOG.info("Failed to switch write intents for Tx. The operation will be retried [txId={}].", txId, ex);
Review Comment:
Don't we need the stacktrace here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2152108556
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/ReplicaManager.java:
##########
@@ -446,7 +449,13 @@ private void handleReplicaRequest(ReplicaRequest request, ClusterNode sender, @N
private static boolean indicatesUnexpectedProblem(Throwable ex) {
Throwable unwrapped = unwrapCause(ex);
return !(unwrapped instanceof ExpectedReplicationException)
- && !hasCause(ex, NodeStoppingException.class, TrackerClosedException.class, ComponentStoppingException.class);
+ && !hasCause(
+ ex,
+ NodeStoppingException.class,
Review Comment:
here is the different set of classes, I wouldn't be sure to change it here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "Cyrill (via GitHub)" <gi...@apache.org>.
Cyrill commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2140012722
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/PlacementDriverMessageProcessor.java:
##########
@@ -151,7 +152,9 @@ CompletableFuture<? extends NetworkMessage> processPlacementDriverMessage(Placem
TimeoutException.class,
ReplicationException.class,
ReplicationTimeoutException.class,
- ReplicaReservationFailedException.class
+ ReplicaReservationFailedException.class,
Review Comment:
same question about ReplicatorRecoverableExceptions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "vldpyatkov (via GitHub)" <gi...@apache.org>.
vldpyatkov commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2142552232
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/WriteIntentSwitchProcessor.java:
##########
@@ -92,8 +92,9 @@ public CompletableFuture<WriteIntentSwitchReplicatedInfo> switchWriteIntentsWith
if (ex != null) {
Throwable cause = ExceptionUtils.unwrapCause(ex);
- if (TransactionFailureHandler.isRecoverable(cause)) {
- LOG.info("Failed to switch write intents for Tx. The operation will be retried [txId={}].", txId, ex);
Review Comment:
We probably shouldn't log this every time here.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -815,8 +811,10 @@ private CompletableFuture<Void> durableFinish(
return CompletableFuture.<Void>failedFuture(cause);
}
- if (TransactionFailureHandler.isRecoverable(cause)) {
- LOG.warn("Failed to finish Tx. The operation will be retried [txId={}].", ex, txId);
+ if (ReplicatorRecoverableExceptions.isRecoverable(cause)) {
+ if (!(cause instanceof GroupOverloadedException)) {
+ LOG.warn("Failed to finish Tx. The operation will be retried [txId={}].", ex, txId);
Review Comment:
Are you sure this should be logged here? These messages flood the log file during overloads.
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/raft/ItCmgRaftServiceTest.java:
##########
@@ -146,7 +146,7 @@ void afterNodeStart() {
RaftGroupService raftService;
if (serverPeer == null) {
- raftService = raftManager.startRaftGroupService(CmgGroupId.INSTANCE, configuration);
+ raftService = raftManager.startRaftGroupService(CmgGroupId.INSTANCE, configuration, true);
Review Comment:
The boolean parameter will never be calculated. It would probably be better to create a specific method to start the system RAFT client, similar to how it was done for the system RAFT node.
##########
modules/raft/src/main/java/org/apache/ignite/internal/raft/Loza.java:
##########
@@ -163,6 +176,16 @@ public Loza(
this.opts = options;
+ double maxInflightOverflowRate = IgniteSystemProperties.getDouble(
Review Comment:
I believe we already agreed to avoid using JVM properties for configuration. RaftConfigurationSchema would be a suitable place for this property instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2152124935
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/PlacementDriverMessageProcessor.java:
##########
@@ -151,7 +152,9 @@ CompletableFuture<? extends NetworkMessage> processPlacementDriverMessage(Placem
TimeoutException.class,
ReplicationException.class,
ReplicationTimeoutException.class,
- ReplicaReservationFailedException.class
+ ReplicaReservationFailedException.class,
Review Comment:
as I said somewhere earlier, this code is not needed at all. I can remove 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
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2152136933
##########
modules/cluster-management/src/integrationTest/java/org/apache/ignite/internal/cluster/management/raft/ItCmgRaftServiceTest.java:
##########
@@ -146,7 +146,7 @@ void afterNodeStart() {
RaftGroupService raftService;
if (serverPeer == null) {
- raftService = raftManager.startRaftGroupService(CmgGroupId.INSTANCE, configuration);
+ raftService = raftManager.startRaftGroupService(CmgGroupId.INSTANCE, configuration, true);
Review Comment:
It's not true, it is used with false. And we already have too many overloaded methods starting raft client that are hard to deal with, so I wouldn't add one more method
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "vldpyatkov (via GitHub)" <gi...@apache.org>.
vldpyatkov merged PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2152105932
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/WriteIntentSwitchProcessor.java:
##########
@@ -92,8 +92,9 @@ public CompletableFuture<WriteIntentSwitchReplicatedInfo> switchWriteIntentsWith
if (ex != null) {
Throwable cause = ExceptionUtils.unwrapCause(ex);
- if (TransactionFailureHandler.isRecoverable(cause)) {
- LOG.info("Failed to switch write intents for Tx. The operation will be retried [txId={}].", txId, ex);
Review Comment:
switched to debug here. Stack trace is not logged
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-25421 Add the requests throttling to raft client [ignite-3]
Posted by "denis-chudov (via GitHub)" <gi...@apache.org>.
denis-chudov commented on code in PR #5915:
URL: https://github.com/apache/ignite-3/pull/5915#discussion_r2152139082
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -815,8 +811,10 @@ private CompletableFuture<Void> durableFinish(
return CompletableFuture.<Void>failedFuture(cause);
}
- if (TransactionFailureHandler.isRecoverable(cause)) {
- LOG.warn("Failed to finish Tx. The operation will be retried [txId={}].", ex, txId);
+ if (ReplicatorRecoverableExceptions.isRecoverable(cause)) {
+ if (!(cause instanceof GroupOverloadedException)) {
+ LOG.warn("Failed to finish Tx. The operation will be retried [txId={}].", ex, txId);
Review Comment:
switched to debug
--
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