NetDB: Lookup handler/throttler fixes (Gitlab #468)

- Allow limited lookups when non-ff, but still disallow expl. lookups - Remove unnecessary banlist checks in handler - Add lower non-ff limit in throttler - Fix check for our RI - Remove now-unused stats
i2p · Nov 9, 2023 · 2d05fe1 · 2d05fe1
1 parent d8415f2
commit 2d05fe1
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 33 deletions.
diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/FloodfillDatabaseLookupMessageHandler.java b/router/java/src/net/i2p/router/networkdb/kademlia/FloodfillDatabaseLookupMessageHandler.java
@@ -35,10 +35,7 @@ public FloodfillDatabaseLookupMessageHandler(RouterContext context, FloodfillNet
         _log = context.logManager().getLog(FloodfillDatabaseLookupMessageHandler.class);
         _context.statManager().createRateStat("netDb.lookupsReceived", "How many netDb lookups have we received?", "NetworkDatabase", new long[] { 60*60*1000l });
         _context.statManager().createRateStat("netDb.lookupsDropped", "How many netDb lookups did we drop due to throttling?", "NetworkDatabase", new long[] { 60*60*1000l });
-        _context.statManager().createRateStat("netDb.lookupsDroppedDueToPriorBan", "How many netDb lookups did we drop due to having a prior ban?", "NetworkDatabase", new long[] { 60*60*1000l });
         _context.statManager().createRateStat("netDb.nonFFLookupsDropped", "How many netDb lookups did we drop due to us not being a floodfill?", "NetworkDatabase", new long[] { 60*60*1000l });
-        _context.statManager().createRateStat("netDb.repeatedLookupsDropped", "How many netDb lookups are coming in faster than we want?", "NetworkDatabase", new long[] { 60*60*1000l });
-        _context.statManager().createRateStat("netDb.repeatedBurstLookupsDropped", "How many netDb lookups did we drop due to burst throttling?", "NetworkDatabase", new long[] { 60*60*1000l });
         // following are for ../HDLMJ
         _context.statManager().createRateStat("netDb.lookupsHandled", "How many netDb lookups have we handled?",
                 "NetworkDatabase", new long[] { 60 * 60 * 1000l });
@@ -65,38 +62,25 @@ public Job createJob(I2NPMessage receivedMessage, RouterIdentity from, Hash from
         _context.statManager().addRateData("netDb.lookupsReceived", 1);
 
         DatabaseLookupMessage dlm = (DatabaseLookupMessage)receivedMessage;
-        boolean isBanned = dlm.getFrom() != null
-                           && (_context.banlist().isBanlistedForever(dlm.getFrom())
-                           || _context.banlist().isBanlisted(dlm.getFrom()));
-        if (isBanned) {
-            _context.statManager().addRateData("netDb.lookupsDroppedDueToPriorBan", 1);
-            return null;
-        }
-        boolean ourRI = dlm.getSearchKey() != null && dlm.getSearchKey().equals(_context.routerHash());
-        if (!_context.netDb().floodfillEnabled() && (dlm.getReplyTunnel() == null && !ourRI)) {
+        if (dlm.getSearchType() == DatabaseLookupMessage.Type.EXPL &&
+            !_context.netDb().floodfillEnabled()) {
             if (_log.shouldLog(Log.WARN)) 
-                _log.warn("[dbid: " + _facade._dbid
+                _log.warn("[dbid: " + _facade
                           + "] Dropping " + dlm.getSearchType()
                           + " lookup request for " + dlm.getSearchKey()
                           + " (we are not a floodfill), reply was to: "
                           + dlm.getFrom() + " tunnel: " + dlm.getReplyTunnel());
             _context.statManager().addRateData("netDb.nonFFLookupsDropped", 1);
             return null;
         }
+
         if (!_facade.shouldThrottleLookup(dlm.getFrom(), dlm.getReplyTunnel())
-                || _context.routerHash().equals(dlm.getFrom())) {
+                || _context.routerHash().equals(dlm.getSearchKey())) {
             Job j = new HandleFloodfillDatabaseLookupMessageJob(_context, dlm, from, fromHash, _msgIDBloomXor);
-            // if (false) {
-            // // might as well inline it, all the heavy lifting is queued up in later jobs,
-            // if necessary
-            // j.runJob();
-            // return null;
-            // } else {
             return j;
-            // }
         } else {
             if (_log.shouldLog(Log.WARN)) 
-                _log.warn("[dbid: " + _facade._dbid
+                _log.warn("[dbid: " + _facade
                           + "] Dropping " + dlm.getSearchType()
                           + " lookup request for " + dlm.getSearchKey()
                           + " (throttled), reply was to: " + dlm.getFrom()

diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/FloodfillNetworkDatabaseFacade.java b/router/java/src/net/i2p/router/networkdb/kademlia/FloodfillNetworkDatabaseFacade.java
@@ -108,7 +108,7 @@ public synchronized void startup() {
             isFF = false;
         } else {
             isFF = _context.getBooleanProperty(FloodfillMonitorJob.PROP_FLOODFILL_PARTICIPANT);
-            _lookupThrottler = new LookupThrottler();
+            _lookupThrottler = new LookupThrottler(this);
         }
 
         long down = _context.router().getEstimatedDowntime();

diff --git a/router/java/src/net/i2p/router/networkdb/kademlia/LookupThrottler.java b/router/java/src/net/i2p/router/networkdb/kademlia/LookupThrottler.java
@@ -19,17 +19,28 @@ class LookupThrottler {
     private final ObjectCounter<ReplyTunnel> counter;
     /** the id of this is -1 */
     private static final TunnelId DUMMY_ID = new TunnelId();
-    /** 30 seems like plenty, possibly too many, maybe dial this down again next release(2.4.0)*/
-    private final int MAX_LOOKUPS; // DEFAULT=20
-    private final long CLEAN_TIME; // DEFAULT=3*60*1000
-    LookupThrottler() {
-        MAX_LOOKUPS = 14;
-        CLEAN_TIME = 2*60*1000;
-        this.counter = new ObjectCounter<ReplyTunnel>();
-        SimpleTimer2.getInstance().addPeriodicEvent(new Cleaner(), CLEAN_TIME);
+    private static final int DEFAULT_MAX_LOOKUPS = 14;
+    private static final int DEFAULT_MAX_NON_FF_LOOKUPS = 3;
+    private static final long DEFAULT_CLEAN_TIME = 2*60*1000;
+    private final int MAX_LOOKUPS;
+    private final int MAX_NON_FF_LOOKUPS;
+    private final long CLEAN_TIME;
+    private final FloodfillNetworkDatabaseFacade _facade;
+    private int _max;
+
+    LookupThrottler(FloodfillNetworkDatabaseFacade facade) {
+        this(facade, DEFAULT_MAX_LOOKUPS, DEFAULT_MAX_NON_FF_LOOKUPS, DEFAULT_CLEAN_TIME);
     }
-    LookupThrottler(int maxlookups, long cleanTime) {
+
+    /**
+     *  @param maxlookups when floodfill
+     *  @param maxnonfflookups when not floodfill
+     *  @since 0.9.60
+     */
+    LookupThrottler(FloodfillNetworkDatabaseFacade facade, int maxlookups, int maxnonfflookups, long cleanTime) {
+        _facade = facade;
         MAX_LOOKUPS = maxlookups;
+        MAX_NON_FF_LOOKUPS = maxnonfflookups;
         CLEAN_TIME = cleanTime;
         this.counter = new ObjectCounter<ReplyTunnel>();
         SimpleTimer2.getInstance().addPeriodicEvent(new Cleaner(), CLEAN_TIME);
@@ -41,12 +52,13 @@ class LookupThrottler {
      * @param id null if for direct lookups
      */
     boolean shouldThrottle(Hash key, TunnelId id) {
-        return this.counter.increment(new ReplyTunnel(key, id)) > MAX_LOOKUPS;
+        return this.counter.increment(new ReplyTunnel(key, id)) > _max;
     }
 
     private class Cleaner implements SimpleTimer.TimedEvent {
         public void timeReached() {
             LookupThrottler.this.counter.clear();
+            _max = _facade.floodfillEnabled() ? MAX_LOOKUPS : MAX_NON_FF_LOOKUPS;
         }
     }