Transport: Don't requeue message after multiple failures

Change failed transports from a Set to a List for efficiency
Check fail count before getting bids
Don't remove from netdb after a failure
Other minor tweaks
This commit is contained in:
zzz
2022-06-07 14:22:27 -04:00
parent ba7b154a09
commit 0d51b2d25f
4 changed files with 40 additions and 15 deletions

View File

@@ -12,10 +12,8 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import net.i2p.data.router.RouterInfo;
import net.i2p.data.i2np.I2NPMessage;
@@ -42,7 +40,7 @@ public class OutNetMessage implements CDPQEntry {
private ReplyJob _onReply;
private Job _onFailedReply;
private MessageSelector _replySelector;
private Set<String> _failedTransports;
private List<String> _failedTransports;
private long _sendBegin;
//private Exception _createdBy;
private final long _created;
@@ -280,14 +278,33 @@ public class OutNetMessage implements CDPQEntry {
public MessageSelector getReplySelector() { return _replySelector; }
public void setReplySelector(MessageSelector selector) { _replySelector = selector; }
public synchronized void transportFailed(String transportStyle) {
if (_failedTransports == null)
_failedTransports = new HashSet<String>(2);
/**
* As of 0.9.55, returns the previous number of failed transports.
*/
public synchronized int transportFailed(String transportStyle) {
int rv;
if (_failedTransports == null) {
_failedTransports = new ArrayList<String>(2);
rv = 0;
} else {
rv = _failedTransports.size();
}
_failedTransports.add(transportStyle);
return rv;
}
public synchronized Set<String> getFailedTransports() {
return (_failedTransports == null ? Collections.<String> emptySet() : _failedTransports);
/**
* @since 0.9.55
*/
public synchronized int getFailedTransportCount() {
return (_failedTransports == null ? 0 : _failedTransports.size());
}
/**
* As of 0.9.55, changed from a Set to a List
*/
public synchronized List<String> getFailedTransports() {
return (_failedTransports == null ? Collections.<String>emptyList() : _failedTransports);
}
/** when did the sending process begin */

View File

@@ -43,6 +43,11 @@ class GetBidsJob extends JobImpl {
}
static void getBids(RouterContext context, TransportManager tmgr, OutNetMessage msg) {
if (msg.getFailedTransportCount() > 1) {
context.statManager().addRateData("transport.bidFailAllTransports", msg.getLifetime());
fail(context, msg);
return;
}
Log log = context.logManager().getLog(GetBidsJob.class);
Hash to = msg.getTarget().getIdentity().getHash();
msg.timestamp("bid");
@@ -67,15 +72,13 @@ class GetBidsJob extends JobImpl {
TransportBid bid = tmgr.getNextBid(msg);
if (bid == null) {
int failedCount = msg.getFailedTransports().size();
int failedCount = msg.getFailedTransportCount();
if (failedCount == 0) {
context.statManager().addRateData("transport.bidFailNoTransports", msg.getLifetime());
// This used to be "no common transports" but it is almost always no transports at all
context.banlist().banlistRouter(to, _x("No transports (hidden or starting up?)"));
} else if (failedCount >= tmgr.getTransportCount()) {
context.statManager().addRateData("transport.bidFailAllTransports", msg.getLifetime());
// fail after all transports were unsuccessful
context.netDb().fail(to);
}
fail(context, msg);
} else {

View File

@@ -310,8 +310,13 @@ public abstract class TransportImpl implements Transport {
else
msg.timestamp("afterSend(failed)");
if (!sendSuccessful)
msg.transportFailed(getStyle());
if (!sendSuccessful) {
int prev = msg.transportFailed(getStyle());
// This catches the usual case with two enabled transports
// GetBidsJob will check against actual transport count
if (prev > 0)
allowRequeue = false;
}
if (msToSend > 1500) {
if (debug)

View File

@@ -847,7 +847,7 @@ public class TransportManager implements TransportEventListener {
throw new IllegalArgumentException("Bids for a message bound to ourselves?");
List<TransportBid> rv = new ArrayList<TransportBid>(_transports.size());
Set<String> failedTransports = msg.getFailedTransports();
List<String> failedTransports = msg.getFailedTransports();
for (Transport t : _transports.values()) {
if (failedTransports.contains(t.getStyle())) {
if (_log.shouldLog(Log.DEBUG))
@@ -873,7 +873,7 @@ public class TransportManager implements TransportEventListener {
TransportBid getNextBid(OutNetMessage msg) {
int unreachableTransports = 0;
Hash peer = msg.getTarget().getIdentity().calculateHash();
Set<String> failedTransports = msg.getFailedTransports();
List<String> failedTransports = msg.getFailedTransports();
TransportBid rv = null;
for (Transport t : _transports.values()) {
if (t.isUnreachable(peer)) {