From fa19ffd0509ccf41f9edb09836f4710105284239 Mon Sep 17 00:00:00 2001 From: onestraw Date: Sun, 20 Jan 2019 17:02:05 +0800 Subject: [PATCH] add @mrd0ll4r 's comments Change-Id: I53616703394f889fa2d0a4e952ac857d99c85218 --- storage/redis/peer_store.go | 44 ++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/storage/redis/peer_store.go b/storage/redis/peer_store.go index 3db858b..e07f43a 100644 --- a/storage/redis/peer_store.go +++ b/storage/redis/peer_store.go @@ -662,6 +662,46 @@ func (ps *peerStore) ScrapeSwarm(ih bittorrent.InfoHash, af bittorrent.AddressFa // // This function must be able to execute while other methods on this interface // are being executed in parallel. +// +// - The Delete(Seeder|Leecher) and GraduateLeecher methods never delete an +// infohash key from an addressFamily hash. They also never decrement the +// infohash counter. +// - The Put(Seeder|Leecher) and GraduateLeecher methods only ever add infohash +// keys to addressFamily hashes and increment the infohash counter. +// - The only method that deletes from the addressFamily hashes is +// collectGarbage, which also decrements the counters. That means that, +// even if a Delete(Seeder|Leecher) call removes the last peer from a swarm, +// the infohash counter is not changed and the infohash is left in the +// addressFamily hash until it will be cleaned up by collectGarbage. +// - collectGarbage must run regularly. +// - A WATCH ... MULTI ... EXEC block fails, if between the WATCH and the 'EXEC' +// any of the watched keys have changed. The location of the 'MULTI' doesn't +// matter. +// +// We have to analyze four cases to prove our algorithm works. I'll characterize +// them by a tuple (number of peers in a swarm before WATCH, number of peers in +// the swarm during the transaction). +// +// 1. (0,0), the easy case: The swarm is empty, we watch the key, we execute +// HLEN and find it empty. We remove it and decrement the counter. It stays +// empty the entire time, the transaction goes through. +// 2. (1,n > 0): The swarm is not empty, we watch the key, we find it non-empty, +// we unwatch the key. All good. No transaction is made, no transaction fails. +// 3. (0,1): We have to analyze this in two ways. +// - If the change happens before the HLEN call, we will see that the swarm is +// not empty and start no transaction. +// - If the change happens after the HLEN, we will attempt a transaction and it +// will fail. This is okay, the swarm is not empty, we will try cleaning it up +// next time collectGarbage runs. +// 4. (1,0): Again, two ways: +// - If the change happens before the HLEN, we will see an empty swarm. This +// situation happens if a call to Delete(Seeder|Leecher) removed the last +// peer asynchronously. We will attempt a transaction, but the transaction +// will fail. This is okay, the infohash key will remain in the addressFamily +// hash, we will attempt to clean it up the next time 'collectGarbage` runs. +// - If the change happens after the HLEN, we will not even attempt to make the +// transaction. The infohash key will remain in the addressFamil hash and +// we'll attempt to clean it up the next time collectGarbage runs. func (ps *peerStore) collectGarbage(cutoff time.Time) error { select { case <-ps.closed: @@ -744,7 +784,9 @@ func (ps *peerStore) collectGarbage(cutoff time.Time) error { conn.Send("MULTI") conn.Send("HDEL", group, ihStr) - conn.Send("DECR", ps.infohashCountKey(group)) + if isSeeder { + conn.Send("DECR", ps.infohashCountKey(group)) + } _, err = redis.Values(conn.Do("EXEC")) if err != nil { log.Error("storage: Redis EXEC failure, maybe caused by WATCH, ignored", log.Fields{