Performance improvement: decrease announce request response time

[josecelano]

Today @WarmBeer and I were discussing a feature to decrease response time.

Introduction

First, I will explain the current process to handle an announce request received by the HTTP tracker.

The HTTP job handles an incoming HTTP request. It could be done in the main job thread or a newly spawned thread.

This is the controller (called handler in Axum):

async fn handle_announce(
    tracker: &Arc<Tracker>,
    announce_request: &Announce,
    client_ip_sources: &ClientIpSources,
    maybe_key: Option<Key>,
) -> Result<AnnounceData, responses::error::Error> {
    // ...

    let peer_ip = match peer_ip_resolver::invoke(tracker.config.on_reverse_proxy, client_ip_sources) {
        Ok(peer_ip) => peer_ip,
        Err(error) => return Err(responses::error::Error::from(error)),
    };

    let mut peer = peer_from_request(announce_request, &peer_ip);

    let announce_data = services::announce::invoke(tracker.clone(), announce_request.info_hash, &mut peer).await;

    Ok(announce_data)
}

I've removed some parts I do not consider relevant to this explanation.

The handler invokes the app service:

pub async fn invoke(tracker: Arc<Tracker>, info_hash: InfoHash, peer: &mut Peer) -> AnnounceData {
    let original_peer_ip = peer.peer_addr.ip();

    let announce_data = tracker.announce(&info_hash, peer, &original_peer_ip).await;

    // Send events ...

    announce_data
}

And the app service invokes the domain tracker service:

    pub async fn announce(&self, info_hash: &InfoHash, peer: &mut Peer, remote_client_ip: &IpAddr) -> AnnounceData {
        peer.change_ip(&assign_ip_address_to_peer(remote_client_ip, self.config.get_ext_ip()));

        let swarm_stats = self.update_torrent_with_peer_and_get_stats(info_hash, peer).await;

        let peers = self.get_peers_for_peer(info_hash, peer).await;

        AnnounceData {
            peers,
            swarm_stats,
            interval: self.config.announce_interval,
            interval_min: self.config.min_announce_interval,
        }
    }

The HTTP tracker protocol performs two operations with one request: a command and a query.

• The command: update the peer list for the torrent
• The query: return the list of peers for the torrent (excluding the peer that has just announce itself)

The announce response contains:

pub struct Announce {
    pub complete: u32,
    pub incomplete: u32,
    pub interval: u32,
    pub min_interval: u32,
    pub peers: Vec<DictionaryPeer>,
}

The peer that makes the announce request is not included in the list, but if the peer has just announced that it has completed downloading the torrent, then the complete attribute would be increased by one.

Alternative solution

It seems we could return a response immediately to the client even before updating the peer list because the peer is not included in the result.

When we receive the request, we could:

• Get the response data
• Build the response
• Send the response immediately to the client
• Continue processing the announce request to update the peer list for the torrent

In fact, updating the peer list and returning the response data could be executed in parallel if we use a different thread.

The main problem to implement this solution is that the response includes the complete statistics that may change if the request event is complete.

That problem has two solutions:

1. We can check if the announce request is for a complete event. In that case, we can process the request as usual. For other cases, we go with the new implementation.
2. The complete does not include the peer making the request in this case. From my point of view, that solution would be less surprising. I would not expect the counter to change if I'm the first peer announcing that torrent. I would expect to get the list of completed peers who are not myself. For my previous research (ChatGPT :-)), it seems we should not count the peer in the seeders counter. So we could totally skip this problem, just returning the stats we get, not including the peer making the request.

So basically, the proposal is: "query command segregation"

Implementation proposal

I think the simplest solution would be changing the service to:

async fn handle_announce(
    tracker: &Arc<Tracker>,
    announce_request: &Announce,
    client_ip_sources: &ClientIpSources,
    maybe_key: Option<Key>,
) -> Result<AnnounceData, responses::error::Error> {
    // ...

    let mut peer = peer_from_request(announce_request, &peer_ip);

    // It returns the current peer list
    let announce_data = services::announce::get_peer_list(tracker.clone(), announce_request.info_hash, &mut peer).await;

    // We do not "await" for this here
    let update_handle = services::announce::update_peer(tracker.clone(), announce_request.info_hash, &mut peer);

    Ok(announce_data)
}

I think we need to spawn a new child because we do not control the web framework, I do not know how we could continue with the update after sending the response. Maybe there is a way to do that with Axum.

We could even improve the solution more if we introduce the second child thread since the beginning: the main thread handles the "query", and the child thread handles the "command". But I think that would not increase the response time because writes and updates have the same priority, I guess. We are using RwLocks. Read more about how the app uses locks here.

Clarifications

This theoretically would improve only the response time, but it's not clear that you can get that benefit with the current app architecture because:

• All announce requests require an update and a query.
• Updates and queries use the same data structure: BTreeMap with locks, which are all sequential.
• If BTreeMap does not have the option to prioritize reads or writes, we assume they will be processed in the other we receive the requests, so the next request has to wait anyway until the previous request finished reading the data form the BTreeMap

Conclusions

It seems that to implement this solution, we should also split the models, having:

• Write models
• Read models

So we could have a queue of pending updates and a queue of pending readings.

We were also talking about a possible implementation for that. We could have a "write" BTreeMap and a "read" BTreeMap. When you write on the write model we trigger an event that updates the read model (cache). We have to have at least two read models:

• One which is being used in a read-only mode
• Another that is mutable.

Every N seconds, we can switch the new cache to read-only and start building the new cache. The problem is the peer list would not be fresh. Clients can miss some peers. So they would get faster responses with fewer peers. The longer you keep the cache, the fewer new peers you get.

Does it make sense?

cc @WarmBeer @da2ce7 @Power2All?

[Power2All]

In my honest opinion, I think a cache for a torrent tracker shouldn't be happening.
It's meant to be dealing with always live and updated data, and with a cache this would undermine the clients from using fresh data.
I would rather you look into something I've been working on, which is making a handler talking with channels, but you could also split up the request. Sadly, HTTP/HTTPS depends on a active connection, so the request should be handled in that request time-frame, with UDP it's a whole different story, you could just hand over the request to a new thread (with a timeout), to create the response and send it back through the cloned connection.

[josecelano]

Hi @Power2All, yes, it seems the channel solution would also be a better framework to implement also this feature.

By the way, I had a meeting today with @da2ce7 and @WarmBeer, and we agreed to migrate to channels. We are currently focused on the next big release: documentation, UI, and testing everything together. But we would like to do it as soon as possible.

Regarding your comment: "HTTP/HTTPS depends on a active connection, so the request should be handled in that request time-frame"

I do not get it. Why can't you do something like the following:

use axum::{handler::get, Router};
use std::sync::Arc;
use tokio::sync::Mutex;

async fn udpate_the_peer_list(some_data: Arc<Mutex<Data>>) {
    // ...
}

async fn announce_handler(tracker: Arc<Tracker>) -> impl IntoResponse {

    tokio::spawn(async move {
        udpate_the_peer_list(tracker.clone()).await;
    });
    
    let announce_data = service::invoke(...);

    // Send a response right away
    Response::new(Body::from("announce data"))
}

#[tokio::main]
async fn main() {

    let app = Router::new().route("/", get(announce_handler));
    //  ...
}

Even if I understand that you could not improve performance because reads/writes go to the same data structure. But I do not see why HTTP is a problem here.

Regarding using a cache, our reason for using a cache was:

• Downloads, on average, take more than a couple of minutes.
• We can refresh the cache every minute.
• The client can get an updated list every minute.
• There should not be too many news peers after only one minute.
• The expiration time could be a settings option.

But I agree that this is "too" aggressive change. I would only do it if we could somehow split reads and writes without duplicating data. That could be easy with channels because you know the nature of the channel message (query or command); we could handle them differently, prioritizing reads, but that would likely lead to an increase in the response time. You would get an accurate peer list, but the response time would be longer.

The announce request is made at intervals of 120 seconds, so the peer does not care if you have new seeders for two minutes. It already has its own 2-minute cache :-/

Anyway, the main point is that we could change the architecture to support splitting reads/writes, so we can easily add caches later if we find out they are helpful and peers do not care about getting 1-minute-old results.