Proposal: add DoNotFulfillResendRequest exception to allow applications to prevent resend

[mgatny]

Adds exception DoNotFulfillResendRequest to fromAdmin, in the spirit of RejectLogon, to alter the behavior of the engine.

Applications can throw this exception to abort the fulfillment of an incoming ResendRequest.

The example use case for this comes from real life, in production at one of the largest derivatives exchanges:

• the counterparty sends an excessive number of large ResendRequests in error (multiple per second)
• this causes a massive spike in cpu, disk, and memory utilization
• there is no way to prevent quickfix from fulfilling the request, other than to log out / disconnect the counterparty
• even if logged out, they will reconnect and repeat the same behavior, causing the same problem with resources
• blocking the counterparty from reconnecting is undersirable: it puts the operational burden on the counterparty experiencing the problem, instead of on the counterparty causing the problem
• the ability to prevent the fulfillment of the ResendRequest allows for keeping the session alive and using it to report the problem via Session-level Reject messages
• if the offending counterparty is able to correct the problem, the session can continue, including resend, without operational intervention by the counterparty on the receiving end of the problem
• this new exception can work in conjunction with existing throttling logic that is in place for other message flow

I know this is a slightly unusual use case, but it's a real one, at a very large institution. This was the best solution we could think of -- if there's a better way to go about it, we'd love to hear your feedback. Thanks!

[chrjohn]

Hi @mgatny , thanks for the PR. Sounds like a sensible enhancement.
At first sight, without digging deeper into the code, the following approach might also work:

Set RejectMessageOnUnhandledException=Y and throw an unhandled exception (e.g. RuntimeException) out of your callback if you receive a ResendRequest. That should also send a session-level reject and increment the target seqnum.

quickfixj/quickfixj-core/src/main/java/quickfix/Session.java

Lines 1201 to 1227 in e612ee6

	} catch (Throwable t) { // QFJ-572
	// If there are any other Throwables we might catch them here if desired.
	// They were most probably thrown out of fromCallback().
	if (rejectMessageOnUnhandledException) {
	getLog().onErrorEvent("Rejecting message: " + t + ": " + getMessageToLog(message));
	if (resetOrDisconnectIfRequired(message)) {
	return;
	}
	if (!(MessageUtils.isAdminMessage(msgType))
	&& (sessionBeginString.compareTo(FixVersions.BEGINSTRING_FIX42) >= 0)) {
	generateBusinessReject(message, BusinessRejectReason.APPLICATION_NOT_AVAILABLE,
	0);
	} else {
	if (MsgType.LOGON.equals(msgType)) {
	disconnect("Problem processing Logon message", true);
	} else {
	generateReject(message, SessionRejectReason.OTHER, 0);
	}
	}
	} else {
	// Re-throw as quickfix.RuntimeError to keep close to the former behaviour
	// and to have a clear notion of what is thrown out of this method.
	// Throwing RuntimeError here means that the target seqnum is not incremented
	// and a resend will be triggered by the next incoming message.
	throw new RuntimeError(t);
	}
	}

One question: are redundant resend requests not ignored? They actually should.
Or are the requests all for a different range of seqnums?

P.S.: Also see #639

[mgatny]

RejectMessageOnUnhandledException=Y is interesting, I didn't know there was such a thing. Maybe instead of a new exception, we could modify that logic slightly so that a reject reason is given (currently it looks like it will give a reason of OTHER with a blank Text field). One (potential) advantage of the new exception is that it lets the Application decide what the behavior should be (e.g. send a Reject, send one of the newer throttling-related messages, etc) but maybe that isn't useful enough to warrant a whole new exception.

Re: redundant resend requests: as far as I know, qf/j does a proper job of not sending them (unless configured to do so), but it fulfills every resend request it receives, even if redundant.

[chrjohn]

I've added RejectMessageOnUnhandledException a couple of years ago since there sometimes were unhandled exceptions, the seqnum was not incremented and one was in an endless loop because the same problematic message was redelivered over and over.
But yes, it would be beneficial to be able to pass a custom text and/or reason. Shall I open an issue for it?

Redundant resend requests: you are right, I mixed that one up.
Another question: the ResendRequests in your initial problem were all for different ranges I suppose? Because otherwise I think one could check if the range for the resend is already being resent and abort the resend processing.

[chrjohn]

Hi @mgatny
is RejectMessageOnUnhandledException good enough for your use case?
Thanks,
Chris

[mgatny]

Hi Chris, still waiting on additional feedback from the client, but for now RejectMessageOnUnhandledException is working for them.

[mgatny]

Hi @chrjohn
I think #851 will resolve the issue, let me know if you think it needs anything else.

[chrjohn]

Superseded by #851