]> granicus.if.org Git - transmission/commitdiff
(trunk libT) #3931 "'Announce is Queued' but doesn't get announced" -- remove the...
authorJordan Lee <jordan@transmissionbt.com>
Sat, 5 Feb 2011 18:46:10 +0000 (18:46 +0000)
committerJordan Lee <jordan@transmissionbt.com>
Sat, 5 Feb 2011 18:46:10 +0000 (18:46 +0000)
The 'bad tracker' penalty was introduced in 2009 after a top tier trackers went down. Announces to it would hang, tying up an announce slot in libcurl for minutes at a time. If a user had enough torrents from that tracker, it could bottleneck all announce slots. The workaround was to deprioritize failing trackers so that they wouldn't obstruct other trackers.

Its implementation could be better, however. There are two parts:

  1. Deciding how frequently to retry unresponsive trackers

  2. Once an unresponsive tracker announce was ready to go, it would be bumped down the queue if other announces were ready too.

Part 2 probably contributes to #3931. If there are enough torrents loaded, there will always be good tracker announces that get pushed ahead of a bad one in the queue. Modifying 2's heuristics would be one option, but it seems simpler to remove it altogether now that getRetryInterval() grades more hashly for consecutive failures. Altering the retry interval also gives better visual feedback to users than Part 2 did.

This commit removes "Part 2" as described above.

libtransmission/announcer.c

index 015cb8bfd554d6601a771f93f3ba65e8d369d3fa..6dc8dd52b6ebe55e2910ae0e9ae58edb3a371631 100644 (file)
@@ -36,7 +36,7 @@
 if( tr_deepLoggingIsActive( ) ) do { \
   char name[128]; \
   tr_snprintf( name, sizeof( name ), "[%s--%s]", tr_torrentName( tier->tor ), \
-      ( tier->currentTracker ? tier->currentTracker->host->name : "" ) ); \
+      ( tier->currentTracker ? tier->currentTracker->hostname : "" ) ); \
   tr_deepLog( __FILE__, __LINE__, name, __VA_ARGS__ ); \
 } while( 0 )
 
@@ -57,16 +57,9 @@ enum
     /* how many web tasks we allow at one time */
     MAX_CONCURRENT_TASKS = 48,
 
-    /* if a tracker takes more than this long to respond,
-     * we treat it as nonresponsive */
-    MAX_TRACKER_RESPONSE_TIME_SECS = ( 60 * 2 ),
-
     /* the value of the 'numwant' argument passed in tracker requests. */
     NUMWANT = 80,
 
-    /* how long to put slow (nonresponsive) trackers in the penalty box */
-    SLOW_HOST_PENALTY_SECS = ( 60 * 10 ),
-
     UPKEEP_INTERVAL_SECS = 1,
 
     /* this is an upper limit for the frequency of LDS announces */
@@ -78,93 +71,6 @@ enum
 ****
 ***/
 
-static int
-compareTransfer( uint64_t a_uploaded, uint64_t a_downloaded,
-                 uint64_t b_uploaded, uint64_t b_downloaded )
-{
-    /* higher upload count goes first */
-    if( a_uploaded != b_uploaded )
-        return a_uploaded > b_uploaded ? -1 : 1;
-
-    /* then higher download count goes first */
-    if( a_downloaded != b_downloaded )
-        return a_downloaded > b_downloaded ? -1 : 1;
-
-    return 0;
-}
-
-/***
-****
-***/
-
-/**
- * used by tr_announcer to recognize nonresponsive
- * trackers and de-prioritize them
- */
-typedef struct
-{
-    char * name;
-
-    /* how many seconds it took to get the last tracker response */
-    int lastResponseInterval;
-
-    /* the last time we sent an announce or scrape message */
-    time_t lastRequestTime;
-
-    /* the last successful announce/scrape time for this host */
-    time_t lastSuccessfulRequest;
-}
-tr_host;
-
-static int
-compareHosts( const void * va, const void * vb )
-{
-    const tr_host * a = va;
-    const tr_host * b = vb;
-    return strcmp( a->name, b->name );
-}
-
-static int
-compareHostToName( const void * va, const void * vb )
-{
-    const tr_host * a = va;
-    return strcmp( a->name, vb );
-}
-
-/* format: hostname + ':' + port */
-static char *
-getHostName( const char * url )
-{
-    int port = 0;
-    char * host = NULL;
-    char * ret;
-    tr_urlParse( url, -1, NULL, &host, &port, NULL );
-    ret = tr_strdup_printf( "%s:%d", ( host ? host : "invalid" ), port );
-    tr_free( host );
-    return ret;
-}
-
-static tr_host*
-hostNew( const char * name )
-{
-    tr_host * host = tr_new0( tr_host, 1 );
-    host->name = tr_strdup( name );
-    return host;
-}
-
-static void
-hostFree( void * vhost )
-{
-    tr_host * host = vhost;
-
-    tr_free( host->name );
-    tr_free( host );
-}
-
-/***
-****
-***/
-
 /**
  * Since we can't poll a tr_torrent's fields after it's destroyed,
  * we pre-build the "stop" announcement message when a torrent
@@ -172,7 +78,6 @@ hostFree( void * vhost )
  */
 struct stop_message
 {
-    tr_host * host;
     char * url;
     uint64_t up;
     uint64_t down;
@@ -185,6 +90,21 @@ stopFree( struct stop_message * stop )
     tr_free( stop );
 }
 
+static int
+compareTransfer( uint64_t a_uploaded, uint64_t a_downloaded,
+                 uint64_t b_uploaded, uint64_t b_downloaded )
+{
+    /* higher upload count goes first */
+    if( a_uploaded != b_uploaded )
+        return a_uploaded > b_uploaded ? -1 : 1;
+
+    /* then higher download count goes first */
+    if( a_downloaded != b_downloaded )
+        return a_downloaded > b_downloaded ? -1 : 1;
+
+    return 0;
+}
+
 static int
 compareStops( const void * va, const void * vb )
 {
@@ -202,7 +122,6 @@ compareStops( const void * va, const void * vb )
  */
 typedef struct tr_announcer
 {
-    tr_ptrArray hosts; /* tr_host */
     tr_ptrArray stops; /* struct stop_message */
     tr_session * session;
     struct event * upkeepTimer;
@@ -217,21 +136,17 @@ tr_announcerHasBacklog( const struct tr_announcer * announcer )
     return announcer->slotsAvailable < 1;
 }
 
-static tr_host *
-getHost( tr_announcer * announcer, const char * url )
+/* format: hostname + ':' + port */
+static char *
+getHostName( const char * url )
 {
-    char * name = getHostName( url );
-    tr_host * host;
-
-    host = tr_ptrArrayFindSorted( &announcer->hosts, name, compareHostToName );
-    if( host == NULL )
-    {
-        host = hostNew( name );
-        tr_ptrArrayInsertSorted( &announcer->hosts, host, compareHosts );
-    }
-
-    tr_free( name );
-    return host;
+    int port = 0;
+    char * host = NULL;
+    char * ret;
+    tr_urlParse( url, -1, NULL, &host, &port, NULL );
+    ret = tr_strdup_printf( "%s:%d", ( host ? host : "invalid" ), port );
+    tr_free( host );
+    return ret;
 }
 
 static inline time_t
@@ -260,7 +175,6 @@ tr_announcerInit( tr_session * session )
     assert( tr_isSession( session ) );
 
     a = tr_new0( tr_announcer, 1 );
-    a->hosts = TR_PTR_ARRAY_INIT;
     a->stops = TR_PTR_ARRAY_INIT;
     a->session = session;
     a->slotsAvailable = MAX_CONCURRENT_TASKS;
@@ -284,7 +198,6 @@ tr_announcerClose( tr_session * session )
     announcer->upkeepTimer = NULL;
 
     tr_ptrArrayDestruct( &announcer->stops, NULL );
-    tr_ptrArrayDestruct( &announcer->hosts, hostFree );
 
     session->announcer = NULL;
     tr_free( announcer );
@@ -297,8 +210,7 @@ tr_announcerClose( tr_session * session )
 /* a row in tr_tier's list of trackers */
 typedef struct
 {
-    tr_host * host;
-
+    char * hostname;
     char * announce;
     char * scrape;
 
@@ -335,13 +247,12 @@ generateKeyParam( unsigned char * msg, size_t msglen )
 }
 
 static tr_tracker_item*
-trackerNew( tr_announcer  * announcer,
-            const char    * announce,
-            const char    * scrape,
-            uint32_t        id )
+trackerNew( const char  * announce,
+            const char  * scrape,
+            uint32_t      id )
 {
     tr_tracker_item * tracker = tr_new0( tr_tracker_item, 1  );
-    tracker->host = getHost( announcer, announce );
+    tracker->hostname = getHostName( announce );
     tracker->announce = tr_strdup( announce );
     tracker->scrape = tr_strdup( scrape );
     tracker->id = id;
@@ -360,6 +271,7 @@ trackerFree( void * vtracker )
     tr_free( tracker->tracker_id );
     tr_free( tracker->scrape );
     tr_free( tracker->announce );
+    tr_free( tracker->hostname );
     tr_free( tracker );
 }
 
@@ -467,13 +379,12 @@ tierIncrementTracker( tr_tier * tier )
 }
 
 static void
-tierAddTracker( tr_announcer * announcer,
-                tr_tier      * tier,
+tierAddTracker( tr_tier      * tier,
                 const char   * announce,
                 const char   * scrape,
                 uint32_t       id )
 {
-    tr_tracker_item * tracker = trackerNew( announcer, announce, scrape, id );
+    tr_tracker_item * tracker = trackerNew( announce, scrape, id );
 
     tr_ptrArrayAppend( &tier->trackers, tracker );
     dbgmsg( tier, "adding tracker %s", announce );
@@ -744,8 +655,7 @@ createAnnounceURL( const tr_announcer     * announcer,
 ***/
 
 static void
-addTorrentToTier( tr_announcer      * announcer,
-                  tr_torrent_tiers  * tiers,
+addTorrentToTier( tr_torrent_tiers  * tiers,
                   tr_torrent        * tor )
 {
     int i, n;
@@ -780,7 +690,7 @@ addTorrentToTier( tr_announcer      * announcer,
                 tr_ptrArrayAppend( &tiers->tiers, tier );
             }
 
-            tierAddTracker( announcer, tier, info->announce, info->scrape, info->id );
+            tierAddTracker( tier, info->announce, info->scrape, info->id );
         }
     }
 
@@ -800,7 +710,7 @@ tr_announcerAddTorrent( tr_announcer * announcer, tr_torrent * tor,
     tiers->callback = callback;
     tiers->callbackData = callbackData;
 
-    addTorrentToTier( announcer, tiers, tor );
+    addTorrentToTier( tiers, tor );
 
     return tiers;
 }
@@ -956,7 +866,6 @@ tr_announcerRemoveTorrent( tr_announcer * announcer, tr_torrent * tor )
                 s->up = tier->byteCounts[TR_ANN_UP];
                 s->down = tier->byteCounts[TR_ANN_DOWN];
                 s->url = createAnnounceURL( announcer, tor, tier, "stopped" );
-                s->host = tier->currentTracker->host;
                 tr_ptrArrayInsertSorted( &announcer->stops, s, compareStops );
             }
         }
@@ -1098,17 +1007,6 @@ parseAnnounceResponse( tr_tier     * tier,
     return success;
 }
 
-struct announce_data
-{
-    int torrentId;
-    int tierId;
-    time_t timeSent;
-    const char * event;
-
-    /** If the request succeeds, the value for tier's "isRunning" flag */
-    tr_bool isRunningOnSuccess;
-};
-
 static int
 getRetryInterval( const tr_tracker_item * t )
 {
@@ -1116,17 +1014,26 @@ getRetryInterval( const tr_tracker_item * t )
     const unsigned int jitter_seconds = tr_cryptoWeakRandInt( 60 );
     switch( t->consecutiveAnnounceFailures ) {
         case 0:  minutes =   1; break;
-        case 1:  minutes =   2; break;
-        case 2:  minutes =   4; break;
-        case 3:  minutes =   8; break;
-        case 4:  minutes =  16; break;
-        case 5:  minutes =  32; break;
-        case 6:  minutes =  64; break;
-        default: minutes = 128; break;
+        case 1:  minutes =   5; break;
+        case 2:  minutes =  15; break;
+        case 3:  minutes =  30; break;
+        case 4:  minutes =  60; break;
+        default: minutes = 120; break;
     }
     return ( minutes * 60 ) + jitter_seconds;
 }
 
+struct announce_data
+{
+    int torrentId;
+    int tierId;
+    time_t timeSent;
+    const char * event;
+
+    /** If the request succeeds, the value for tier's "isRunning" flag */
+    tr_bool isRunningOnSuccess;
+};
+
 static void
 onAnnounceDone( tr_session   * session,
                 long           responseCode,
@@ -1151,16 +1058,8 @@ onAnnounceDone( tr_session   * session,
         tier->manualAnnounceAllowedAt = now + tier->announceMinIntervalSec;
 
         if(( tracker = tier->currentTracker ))
-        {
             ++tracker->consecutiveAnnounceFailures;
 
-            if( tracker->host )
-            {
-                tracker->host->lastRequestTime = data->timeSent;
-                tracker->host->lastResponseInterval = now - data->timeSent;
-            }
-        }
-
         if( responseCode == HTTP_OK )
         {
             tr_bool gotScrape;
@@ -1174,9 +1073,6 @@ onAnnounceDone( tr_session   * session,
                 if(( tracker = tier->currentTracker ))
                 {
                     tracker->consecutiveAnnounceFailures = 0;
-    
-                    if( tracker->host )
-                        tracker->host->lastSuccessfulRequest = now;
                 }
 
                 if( gotScrape )
@@ -1431,13 +1327,6 @@ onScrapeDone( tr_session   * session,
         tier->isScraping = FALSE;
         tier->lastScrapeTime = now;
 
-        if( tier->currentTracker->host )
-        {
-            tr_host * host = tier->currentTracker->host;
-            host->lastRequestTime = data->timeSent;
-            host->lastResponseInterval = now - data->timeSent;
-        }
-
         if( 200 <= responseCode && responseCode <= 299 )
         {
             const int interval = tier->scrapeIntervalSec;
@@ -1485,9 +1374,6 @@ onScrapeDone( tr_session   * session,
 
         tier->lastScrapeSucceeded = success;
         tier->lastScrapeTimedOut = responseCode == 0;
-
-        if( success && tier->currentTracker->host )
-            tier->currentTracker->host->lastSuccessfulRequest = now;
     }
 
     tr_free( data );
@@ -1561,73 +1447,24 @@ tierNeedsToScrape( const tr_tier * tier, const time_t now )
         && ( tier->currentTracker->scrape != NULL );
 }
 
-/* return true if (1) we've tried it recently AND (2) it didn't respond... */
-static tr_bool
-hostIsNotResponding( const tr_host * host, const time_t now )
-{
-    tr_bool b = ( host->lastRequestTime )
-             && ( host->lastRequestTime >= ( now - SLOW_HOST_PENALTY_SECS ) )
-             && ( host->lastResponseInterval > MAX_TRACKER_RESPONSE_TIME_SECS );
-    return b;
-}
-
-static tr_bool
-tierIsNotResponding( const tr_tier * tier, const time_t now )
-{
-    return !tier->currentTracker
-        || hostIsNotResponding( tier->currentTracker->host, now );
-}
-
 static int
 compareTiers( const void * va, const void * vb )
 {
-    int ret = 0;
-    tr_bool af, bf;
+    int ret;
     const tr_tier * a = *(const tr_tier**)va;
     const tr_tier * b = *(const tr_tier**)vb;
 
-    /* working domains come before non-working */
-    if( !ret ) {
-        const time_t now = tr_time( );
-        af = tierIsNotResponding( a, now );
-        bf = tierIsNotResponding( b, now );
-        if( af != bf )
-            ret = !af ? -1 : 1;
-    }
-
-    /* stops come before starts */
-    if( !ret ) {
-        af = a->tor->isRunning;
-        bf = b->tor->isRunning;
-        if( af != bf )
-            ret = af ? 1 : -1;
-    }
-
-    /* upload comes before download */
-    if( !ret )
-        ret = compareTransfer( a->byteCounts[TR_ANN_UP], a->byteCounts[TR_ANN_DOWN],
-                               b->byteCounts[TR_ANN_UP], b->byteCounts[TR_ANN_DOWN] );
-
-    /* incomplete comes before complete */
-    if( !ret ) {
-        af = a->tor->completeness == TR_LEECH;
-        bf = b->tor->completeness == TR_LEECH;
-        if( af != bf )
-            return af ? -1 : 1;
-    }
-
-    /* private before public */
-    if( !ret ) {
-        af = tr_torrentIsPrivate( a->tor );
-        bf = tr_torrentIsPrivate( b->tor );
-        if( af != bf )
-            ret = af ? -1 : 1;
-    }
+    /* primary key: larger stats come before smaller */
+    ret = compareTransfer( a->byteCounts[TR_ANN_UP], a->byteCounts[TR_ANN_DOWN],
+                           b->byteCounts[TR_ANN_UP], b->byteCounts[TR_ANN_DOWN] );
 
+    /* secondary key: announcements that have been waiting longer go first */
+    if( !ret && ( a->announceAt != b->announceAt ) )
+        ret = a->announceAt < b->announceAt ? -1 : 1;
+         
     return ret;
 }
 
-
 static void
 announceMore( tr_announcer * announcer )
 {
@@ -1785,7 +1622,7 @@ tr_announcerStats( const tr_torrent * torrent,
             tr_tracker_stat * st = ret + out++;
 
             st->id = tracker->id;
-            tr_strlcpy( st->host, tracker->host->name, sizeof( st->host ) );
+            tr_strlcpy( st->host, tracker->hostname, sizeof( st->host ) );
             tr_strlcpy( st->announce, tracker->announce, sizeof( st->announce ) );
             st->tier = i;
             st->isBackup = tracker != tier->currentTracker;
@@ -1904,7 +1741,7 @@ tierCopyAttributes( tr_tier * t, const tr_tier * o )
 }
 
 void
-tr_announcerResetTorrent( tr_announcer * announcer, tr_torrent * tor )
+tr_announcerResetTorrent( tr_announcer * announcer UNUSED, tr_torrent * tor )
 {
     tr_ptrArray oldTiers = TR_PTR_ARRAY_INIT;
 
@@ -1916,7 +1753,7 @@ tr_announcerResetTorrent( tr_announcer * announcer, tr_torrent * tor )
     }
 
     /* create the new tier/tracker structs */
-    addTorrentToTier( announcer, tor->tiers, tor );
+    addTorrentToTier( tor->tiers, tor );
 
     /* if we had tiers already, merge their state into the new structs */
     if( !tr_ptrArrayEmpty( &oldTiers ) )