From cd6e4abec08e1e35cb35242b0410c4682bdba11c Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Fri, 6 Sep 2013 20:59:12 -0400 Subject: [PATCH 1/9] Implemented redis cache driver using redis types --- cache/cache.go | 4 + cache/redis/redis.go | 428 +++++++++++++++++++++++++------------- cache/redis/redis_test.go | 356 +++++++------------------------ models/models.go | 1 + 4 files changed, 362 insertions(+), 427 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index 569d9c7..e477ede 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -82,4 +82,8 @@ type Tx interface { SetSeeder(t *models.Torrent, p *models.Peer) error IncrementSlots(u *models.User) error DecrementSlots(u *models.User) error + AddTorrent(t *models.Torrent) error + RemoveTorrent(t *models.Torrent) error + AddUser(u *models.User) error + RemoveUser(u *models.User) error } diff --git a/cache/redis/redis.go b/cache/redis/redis.go index 21ae0ea..7281f0f 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -14,7 +14,8 @@ package redis import ( - "encoding/json" + "errors" + "strconv" "strings" "time" @@ -25,6 +26,18 @@ import ( "github.com/pushrax/chihaya/models" ) +var ( + ErrCreateUser = errors.New("redis: Incorrect reply length for user") + ErrCreateTorrent = errors.New("redis: Incorrect reply length for torrent") + ErrCreatePeer = errors.New("redis: Incorrect reply length for peer") + ErrMarkActive = errors.New("redis: Torrent doesn't exist") + + SeederSuffix = ":seeders" + LeecherSuffix = ":leechers" + TorrentPrefix = "torrent:" + UserPrefix = "user:" +) + type driver struct{} func (d *driver) New(conf *config.DataStore) cache.Pool { @@ -65,27 +78,12 @@ func (p *Pool) Close() error { func (p *Pool) Get() (cache.Tx, error) { return &Tx{ - conf: p.conf, - done: false, - multi: false, - Conn: p.pool.Get(), + conf: p.conf, + done: false, + Conn: p.pool.Get(), }, nil } -// Tx represents a transaction for Redis with one gotcha: -// all reads must be done prior to any writes. Writes will -// check if the MULTI command has been sent to redis and will -// send it if it hasn't. -// -// Internally a transaction looks like: -// WATCH keyA -// GET keyA -// WATCH keyB -// GET keyB -// MULTI -// SET keyA -// SET keyB -// EXEC type Tx struct { conf *config.DataStore done bool @@ -101,27 +99,6 @@ func (tx *Tx) close() { tx.Conn.Close() } -func (tx *Tx) initiateWrite() error { - if tx.done { - return cache.ErrTxDone - } - if tx.multi != true { - tx.multi = true - return tx.Send("MULTI") - } - return nil -} - -func (tx *Tx) initiateRead() error { - if tx.done { - return cache.ErrTxDone - } - if tx.multi == true { - panic("Tried to read during MULTI") - } - return nil -} - func (tx *Tx) Commit() error { if tx.done { return cache.ErrTxDone @@ -153,163 +130,324 @@ func (tx *Tx) Rollback() error { return nil } -func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { - err := tx.initiateRead() - if err != nil { - return nil, false, err +func createUser(userVals []string) (*models.User, error) { + if len(userVals) != 7 { + return nil, ErrCreateUser } - - key := tx.conf.Prefix + "user:" + passkey - _, err = tx.Do("WATCH", key) + // This could be a loop+switch + ID, err := strconv.ParseUint(userVals[0], 10, 64) if err != nil { - return nil, false, err + return nil, err } - reply, err := redis.String(tx.Do("GET", key)) + Passkey := userVals[1] + UpMultiplier, err := strconv.ParseFloat(userVals[2], 64) if err != nil { - if err == redis.ErrNil { - return nil, false, nil - } - return nil, false, err + return nil, err } - - user := &models.User{} - err = json.NewDecoder(strings.NewReader(reply)).Decode(user) + DownMultiplier, err := strconv.ParseFloat(userVals[3], 64) if err != nil { - return nil, true, err + return nil, err } - return user, true, nil + Slots, err := strconv.ParseInt(userVals[4], 10, 64) + if err != nil { + return nil, err + } + SlotsUsed, err := strconv.ParseInt(userVals[5], 10, 64) + if err != nil { + return nil, err + } + Snatches, err := strconv.ParseUint(userVals[6], 10, 64) + if err != nil { + return nil, err + } + return &models.User{ID, Passkey, UpMultiplier, DownMultiplier, Slots, SlotsUsed, uint(Snatches)}, nil } -func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { - err := tx.initiateRead() - if err != nil { - return nil, false, err +func createTorrent(torrentVals []string, seeders map[string]models.Peer, leechers map[string]models.Peer) (*models.Torrent, error) { + if len(torrentVals) != 7 { + return nil, ErrCreateTorrent } + ID, err := strconv.ParseUint(torrentVals[0], 10, 64) + if err != nil { + return nil, err + } + Infohash := torrentVals[1] + Active, err := strconv.ParseBool(torrentVals[2]) + if err != nil { + return nil, err + } + Snatches, err := strconv.ParseUint(torrentVals[3], 10, 32) + if err != nil { + return nil, err + } + UpMultiplier, err := strconv.ParseFloat(torrentVals[4], 64) + if err != nil { + return nil, err + } + DownMultiplier, err := strconv.ParseFloat(torrentVals[5], 64) + if err != nil { + return nil, err + } + LastAction, err := strconv.ParseInt(torrentVals[6], 10, 64) + if err != nil { + return nil, err + } + return &models.Torrent{ID, Infohash, Active, seeders, leechers, uint(Snatches), UpMultiplier, DownMultiplier, LastAction}, nil - key := tx.conf.Prefix + "torrent:" + infohash - _, err = tx.Do("WATCH", key) - if err != nil { - return nil, false, err - } - reply, err := redis.String(tx.Do("GET", key)) - if err != nil { - if err == redis.ErrNil { - return nil, false, nil - } - return nil, false, err - } - - torrent := &models.Torrent{} - err = json.NewDecoder(strings.NewReader(reply)).Decode(torrent) - if err != nil { - return nil, true, err - } - return torrent, true, nil } -func (tx *Tx) ClientWhitelisted(peerID string) (exists bool, err error) { - err = tx.initiateRead() +// Prevents adding duplicate peers, and doesn't return error on dup add +func (tx *Tx) addPeer(infohash string, peer *models.Peer, suffix string) error { + setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix + _, err := tx.Do("SADD", setKey, *peer) if err != nil { - return false, err - } - - key := tx.conf.Prefix + "whitelist" - _, err = tx.Do("WATCH", key) - if err != nil { - return - } - - // TODO - return -} - -func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { - if err := tx.initiateWrite(); err != nil { return err } - - // TODO return nil } -func (tx *Tx) MarkActive(t *models.Torrent) error { - if err := tx.initiateWrite(); err != nil { +// Will not return an error if the peer doesn't exist +func (tx *Tx) removePeer(infohash string, peer *models.Peer, suffix string) error { + setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix + _, err := tx.Do("SREM", setKey, *peer) + if err != nil { + return err + } + return nil +} + +func (tx *Tx) addPeers(infohash string, peers map[string]models.Peer, suffix string) error { + setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix + for _, peer := range peers { + err := tx.Send("SADD", setKey, peer) + if err != nil { + return err + } + } + tx.Flush() + tx.Receive() + return nil +} + +func createPeer(peerString string) (*models.Peer, error) { + peerVals := strings.Split(strings.Trim(peerString, "{}"), " ") + if len(peerVals) != 9 { + return nil, ErrCreatePeer + } + ID := peerVals[0] + UserID, err := strconv.ParseUint(peerVals[1], 10, 64) + if err != nil { + return nil, err + } + TorrentID, err := strconv.ParseUint(peerVals[2], 10, 64) + if err != nil { + return nil, err + } + IP := peerVals[3] + + Port, err := strconv.ParseUint(peerVals[4], 10, 64) + if err != nil { + return nil, err + } + Uploaded, err := strconv.ParseUint(peerVals[5], 10, 64) + if err != nil { + return nil, err + } + Downloaded, err := strconv.ParseUint(peerVals[6], 10, 64) + if err != nil { + return nil, err + } + Left, err := strconv.ParseUint(peerVals[7], 10, 64) + if err != nil { + return nil, err + } + LastAnnounce, err := strconv.ParseInt(peerVals[8], 10, 64) + if err != nil { + return nil, err + } + return &models.Peer{ID, UserID, TorrentID, IP, Port, Uploaded, Downloaded, Left, LastAnnounce}, nil + +} + +func (tx *Tx) getPeers(infohash string, suffix string) (peers map[string]models.Peer, err error) { + peers = make(map[string]models.Peer) + setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix + peerStrings, err := redis.Strings(tx.Do("SMEMBERS", setKey)) + for peerIndex := range peerStrings { + peer, err := createPeer(peerStrings[peerIndex]) + if err != nil { + return nil, err + } + peers[peer.ID] = *peer + } + return +} + +func (tx *Tx) AddTorrent(t *models.Torrent) error { + hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash + _, err := tx.Do("HMSET", hashkey, + "id", t.ID, + "infohash", t.Infohash, + "active", t.Active, + "snatches", t.Snatches, + "up_multiplier", t.UpMultiplier, + "down_multiplier", t.DownMultiplier, + "last_action", t.LastAction) + if err != nil { return err } - // TODO + tx.addPeers(t.Infohash, t.Seeders, SeederSuffix) + tx.addPeers(t.Infohash, t.Leechers, LeecherSuffix) + + return nil +} + +func (tx *Tx) RemoveTorrent(t *models.Torrent) error { + hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash + _, err := tx.Do("HDEL", hashkey) + if err != nil { + return err + } + return nil +} + +func (tx *Tx) AddUser(u *models.User) error { + hashkey := tx.conf.Prefix + UserPrefix + u.Passkey + _, err := tx.Do("HMSET", hashkey, + "id", u.ID, + "passkey", u.Passkey, + "up_multiplier", u.UpMultiplier, + "down_multiplier", u.DownMultiplier, + "slots", u.Slots, + "slots_used", u.SlotsUsed, + "snatches", u.Snatches) + if err != nil { + return err + } + return nil +} + +func (tx *Tx) RemoveUser(u *models.User) error { + hashkey := tx.conf.Prefix + UserPrefix + u.Passkey + _, err := tx.Do("HDEL", hashkey) + if err != nil { + return err + } + return nil +} + +func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { + hashkey := tx.conf.Prefix + UserPrefix + passkey + userStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) + if err != nil { + return nil, false, err + } else if len(userStrings) == 0 { + return nil, false, nil + } + foundUser, err := createUser(userStrings) + if err != nil { + return nil, false, err + } + return foundUser, true, nil +} + +func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { + hashkey := tx.conf.Prefix + TorrentPrefix + infohash + torrentStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) + if err != nil { + return nil, false, err + } else if len(torrentStrings) == 0 { + return nil, false, nil + } + + seeders, err := tx.getPeers(infohash, SeederSuffix) + leechers, err := tx.getPeers(infohash, LeecherSuffix) + foundTorrent, err := createTorrent(torrentStrings, seeders, leechers) + if err != nil { + return nil, false, err + } + return foundTorrent, true, nil +} + +func (tx *Tx) ClientWhitelisted(peerID string) (exists bool, err error) { + key := tx.conf.Prefix + "whitelist" + return redis.Bool(tx.Do("ISMEMBER", key, peerID)) +} + +// This is a mulple action command, it's not internally atomic +func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { + + torrentKey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash + snatchCount, err := redis.Int(tx.Do("HINCRBY", torrentKey, 1)) + if err != nil { + return err + } + torrent.Snatches = uint(snatchCount) + + userKey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash + snatchCount, err = redis.Int(tx.Do("HINCRBY", userKey, 1)) + if err != nil { + return err + } + user.Snatches = uint(snatchCount) + return nil +} + +func (tx *Tx) MarkActive(torrent *models.Torrent) error { + hashkey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash + activeExists, err := redis.Int(tx.Do("HSET", hashkey, true)) + if err != nil { + return err + } + // HSET returns 1 if hash didn't exist before + if activeExists == 1 { + return ErrMarkActive + } return nil } func (tx *Tx) AddLeecher(t *models.Torrent, p *models.Peer) error { - if err := tx.initiateWrite(); err != nil { - return err - } - - // TODO - return nil + return tx.addPeer(t.Infohash, p, LeecherSuffix) } func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { - if err := tx.initiateWrite(); err != nil { - return err - } - - // TODO - return nil + return tx.addPeer(t.Infohash, p, LeecherSuffix) } func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { - if err := tx.initiateWrite(); err != nil { - return err - } - - // TODO - return nil + return tx.removePeer(t.Infohash, p, LeecherSuffix) } func (tx *Tx) AddSeeder(t *models.Torrent, p *models.Peer) error { - if err := tx.initiateWrite(); err != nil { - return err - } - - // TODO - return nil + return tx.addPeer(t.Infohash, p, SeederSuffix) } func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { - if err := tx.initiateWrite(); err != nil { - return err - } - - // TODO - return nil + return tx.addPeer(t.Infohash, p, SeederSuffix) } func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { - if err := tx.initiateWrite(); err != nil { - return err - } - - // TODO - return nil + return tx.removePeer(t.Infohash, p, SeederSuffix) } func (tx *Tx) IncrementSlots(u *models.User) error { - if err := tx.initiateWrite(); err != nil { + hashkey := tx.conf.Prefix + UserPrefix + u.Passkey + slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, 1)) + if err != nil { return err } - - // TODO + u.Slots = int64(slotCount) return nil } func (tx *Tx) DecrementSlots(u *models.User) error { - if err := tx.initiateWrite(); err != nil { + hashkey := tx.conf.Prefix + UserPrefix + u.Passkey + slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, -1)) + if err != nil { return err } - - // TODO + u.Slots = int64(slotCount) return nil } diff --git a/cache/redis/redis_test.go b/cache/redis/redis_test.go index f1a3cd4..3a18ea1 100644 --- a/cache/redis/redis_test.go +++ b/cache/redis/redis_test.go @@ -5,13 +5,8 @@ package redis import ( - "encoding/json" - "math/rand" "os" - "strconv" - "strings" "testing" - "time" "github.com/garyburd/redigo/redis" @@ -22,6 +17,7 @@ import ( // Maximum number of parallel retries; depends on system latency const MAX_RETRIES = 9000 + const sample_infohash = "58c290f4ea1efb3adcb8c1ed2643232117577bcd" const sample_passkey = "32426b162be0bce5428e7e36afaf734ae5afb355" @@ -39,6 +35,28 @@ func verifyErrNil(err error, t TestReporter) { } } +// Legacy JSON support for benching +func (tx *Tx) initiateWrite() error { + if tx.done { + return cache.ErrTxDone + } + if tx.multi != true { + tx.multi = true + return tx.Send("MULTI") + } + return nil +} + +func (tx *Tx) initiateRead() error { + if tx.done { + return cache.ErrTxDone + } + if tx.multi == true { + panic("Tried to read during MULTI") + } + return nil +} + func createTestTxObj(t TestReporter) *Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) conf := &testConfig.Cache @@ -71,22 +89,8 @@ func createTestTxObj(t TestReporter) *Tx { return txObj } -func createUserFromValues(userVals []string, t TestReporter) *models.User { - ID, err := strconv.ParseUint(userVals[0], 10, 64) - verifyErrNil(err, t) - Passkey := userVals[1] - UpMultiplier, err := strconv.ParseFloat(userVals[2], 64) - verifyErrNil(err, t) - DownMultiplier, err := strconv.ParseFloat(userVals[3], 64) - Slots, err := strconv.ParseInt(userVals[4], 10, 64) - verifyErrNil(err, t) - SlotsUsed, err := strconv.ParseInt(userVals[5], 10, 64) - verifyErrNil(err, t) - return &models.User{ID, Passkey, UpMultiplier, DownMultiplier, Slots, SlotsUsed} -} - -func createUser() models.User { - testUser := models.User{214, "32426b162be0bce5428e7e36afaf734ae5afb355", 0.0, 0.0, 4, 2} +func createTestUser() models.User { + testUser := models.User{214, "32426b162be0bce5428e7e36afaf734ae5afb355", 1.01, 1.0, 4, 2, 7} return testUser } @@ -105,7 +109,7 @@ func createLeechers() []models.Peer { return testLeechers } -func createTorrent() models.Torrent { +func createTestTorrent() models.Torrent { testSeeders := createSeeders() testLeechers := createLeechers() @@ -124,96 +128,6 @@ func createTorrent() models.Torrent { return testTorrent } -func createTorrentJson(t TestReporter) []byte { - jsonTorrent, err := json.Marshal(createTorrent()) - verifyErrNil(err, t) - return jsonTorrent -} - -func createUserJson(t TestReporter) []byte { - jsonUser, err := json.Marshal(createUser()) - verifyErrNil(err, t) - return jsonUser -} - -func ExampleJsonTransaction(testTx *Tx, retries int, t TestReporter) { - defer func() { - if err := recover(); err != nil { - t.Error(err) - } - }() - verifyErrNil(testTx.initiateRead(), t) - _, err := testTx.Do("WATCH", "testKeyA") - verifyErrNil(err, t) - - _, err = redis.String(testTx.Do("GET", "testKeyA")) - if err != nil { - if err == redis.ErrNil { - t.Log("testKeyA does not exist yet") - } else { - t.Error(err) - } - } - _, err = testTx.Do("WATCH", "testKeyB") - verifyErrNil(err, t) - - _, err = redis.String(testTx.Do("GET", "testKeyB")) - if err != nil { - if err == redis.ErrNil { - t.Log("testKeyB does not exist yet") - } else { - t.Error(err) - } - } - - verifyErrNil(testTx.initiateWrite(), t) - - // Generate random data to set - randGen := rand.New(rand.NewSource(time.Now().UnixNano())) - verifyErrNil(testTx.Send("SET", "testKeyA", strconv.Itoa(randGen.Int())), t) - verifyErrNil(testTx.Send("SET", "testKeyB", strconv.Itoa(randGen.Int())), t) - - err = testTx.Commit() - // For parallel runs, there may be conflicts, retry until successful - if err == cache.ErrTxConflict && retries > 0 { - ExampleJsonTransaction(testTx, retries-1, t) - // Clear TxConflict, if retries max out, errors are already recorded - err = nil - } else if err == cache.ErrTxConflict { - t.Error("Conflict encountered, max retries reached") - t.Error(err) - } - verifyErrNil(err, t) -} - -func ExampleJsonSchemaRemoveSeeder(torrent *models.Torrent, peer *models.Peer, t TestReporter) { - testTx := createTestTxObj(t) - - verifyErrNil(testTx.initiateRead(), t) - - key := testTx.conf.Prefix + "torrent:" + torrent.Infohash - _, err := testTx.Do("WATCH", key) - reply, err := redis.String(testTx.Do("GET", key)) - if err != nil { - if err == redis.ErrNil { - t.Error("testTorrent does not exist") - } else { - t.Error(err) - } - } - - verifyErrNil(json.NewDecoder(strings.NewReader(reply)).Decode(torrent), t) - - delete(torrent.Seeders, "testPeerID2") - - jsonTorrent, err := json.Marshal(torrent) - verifyErrNil(err, t) - verifyErrNil(testTx.initiateWrite(), t) - verifyErrNil(testTx.Send("SET", key, jsonTorrent), t) - verifyErrNil(testTx.Commit(), t) - -} - func ExampleRedisTypeSchemaRemoveSeeder(torrent *models.Torrent, peer *models.Peer, t TestReporter) { testTx := createTestTxObj(t) setkey := testTx.conf.Prefix + "torrent:" + torrent.Infohash + ":seeders" @@ -224,64 +138,70 @@ func ExampleRedisTypeSchemaRemoveSeeder(torrent *models.Torrent, peer *models.Pe verifyErrNil(err, t) } -func ExampleJsonSchemaFindUser(passkey string, t TestReporter) (*models.User, bool) { - testTx := createTestTxObj(t) - - verifyErrNil(testTx.initiateRead(), t) - - key := testTx.conf.Prefix + "user:" + passkey - _, err := testTx.Do("WATCH", key) - verifyErrNil(err, t) - reply, err := redis.String(testTx.Do("GET", key)) - if err != nil { - if err == redis.ErrNil { - return nil, false - } else { - t.Error(err) - } - } - - user := &models.User{} - verifyErrNil(json.NewDecoder(strings.NewReader(reply)).Decode(user), t) - return user, true -} - func ExampleRedisTypesSchemaFindUser(passkey string, t TestReporter) (*models.User, bool) { testTx := createTestTxObj(t) - hashkey := testTx.conf.Prefix + "user_hash:" + sample_passkey + hashkey := testTx.conf.Prefix + UserPrefix + passkey userVals, err := redis.Strings(testTx.Do("HVALS", hashkey)) - if userVals == nil { + if len(userVals) == 0 { return nil, false } verifyErrNil(err, t) - compareUser := createUserFromValues(userVals, t) + compareUser, err := createUser(userVals) + verifyErrNil(err, t) return compareUser, true } -func BenchmarkRedisJsonSchemaRemoveSeeder(b *testing.B) { - for bCount := 0; bCount < b.N; bCount++ { - b.StopTimer() - testTx := createTestTxObj(b) - testTorrent := createTorrent() - testSeeders := createSeeders() - key := testTx.conf.Prefix + "torrent:" + testTorrent.Infohash - // Benchmark setup not a transaction, not thread-safe - _, err := testTx.Do("SET", key, createTorrentJson(b)) - verifyErrNil(err, b) - b.StartTimer() +func TestFindUserSuccess(t *testing.T) { + testUser := createTestUser() + testTx := createTestTxObj(t) + hashkey := testTx.conf.Prefix + UserPrefix + sample_passkey + _, err := testTx.Do("DEL", hashkey) + verifyErrNil(err, t) - ExampleJsonSchemaRemoveSeeder(&testTorrent, &testSeeders[2], b) + err = testTx.AddUser(&testUser) + verifyErrNil(err, t) + + compareUser, exists := ExampleRedisTypesSchemaFindUser(sample_passkey, t) + + if !exists { + t.Error("User not found!") + } + if testUser != *compareUser { + t.Errorf("user mismatch: %v vs. %v", compareUser, testUser) + } +} + +func TestFindUserFail(t *testing.T) { + compareUser, exists := ExampleRedisTypesSchemaFindUser("not_a_user_passkey", t) + if exists { + t.Errorf("User %v found when none should exist!", compareUser) + } +} + +func TestAddGetPeers(t *testing.T) { + + testTx := createTestTxObj(t) + testTorrent := createTestTorrent() + + setkey := testTx.conf.Prefix + "torrent:" + testTorrent.Infohash + ":seeders" + testTx.Do("DEL", setkey) + + testTx.addPeers(testTorrent.Infohash, testTorrent.Seeders, ":seeders") + peerMap, err := testTx.getPeers(sample_infohash, ":seeders") + if err != nil { + t.Error(err) + } else if len(peerMap) != len(testTorrent.Seeders) { + t.Error("Num Peers not equal") } } func BenchmarkRedisTypesSchemaRemoveSeeder(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { - // Ensure that remove completes successfully, // even if it doesn't impact the performance b.StopTimer() testTx := createTestTxObj(b) - testTorrent := createTorrent() + testTorrent := createTestTorrent() setkey := testTx.conf.Prefix + "torrent:" + testTorrent.Infohash + ":seeders" testSeeders := createSeeders() reply, err := redis.Int(testTx.Do("SADD", setkey, @@ -300,37 +220,13 @@ func BenchmarkRedisTypesSchemaRemoveSeeder(b *testing.B) { } } -func BenchmarkRedisJsonSchemaFindUser(b *testing.B) { - // Ensure successful user find ( a failed lookup may have different performance ) - b.StopTimer() - testTx := createTestTxObj(b) - testUser := createUser() - userJson := string(createUserJson(b)) - verifyErrNil(testTx.initiateWrite(), b) - key := testTx.conf.Prefix + "user:" + sample_passkey - verifyErrNil(testTx.Send("SET", key, userJson), b) - verifyErrNil(testTx.Commit(), b) - b.StartTimer() - for bCount := 0; bCount < b.N; bCount++ { - compareUser, exists := ExampleJsonSchemaFindUser(sample_passkey, b) - b.StopTimer() - if !exists { - b.Error("User not found!") - } - if testUser != *compareUser { - b.Errorf("user mismatch: %v vs. %v", compareUser, testUser) - } - b.StartTimer() - } -} - func BenchmarkRedisTypesSchemaFindUser(b *testing.B) { // Ensure successful user find ( a failed lookup may have different performance ) b.StopTimer() - testUser := createUser() + testUser := createTestUser() testTx := createTestTxObj(b) - hashkey := testTx.conf.Prefix + "user_hash:" + sample_passkey + hashkey := testTx.conf.Prefix + UserPrefix + sample_passkey reply, err := testTx.Do("HMSET", hashkey, "id", testUser.ID, "passkey", testUser.Passkey, @@ -340,7 +236,7 @@ func BenchmarkRedisTypesSchemaFindUser(b *testing.B) { "slots_used", testUser.SlotsUsed) if reply == nil { - b.Error("no hash fields added!") + b.Log("no hash fields added!") } verifyErrNil(err, b) b.StartTimer() @@ -360,13 +256,6 @@ func BenchmarkRedisTypesSchemaFindUser(b *testing.B) { } } -func TestRedisTransaction(t *testing.T) { - for i := 0; i < 10; i++ { - // No retries for serial transactions - ExampleJsonTransaction(createTestTxObj(t), 0, t) - } -} - func TestReadAfterWrite(t *testing.T) { // Test requires panic defer func() { @@ -392,100 +281,3 @@ func TestCloseClosedTransaction(t *testing.T) { testTx.close() testTx.close() } - -func TestParallelTx0(t *testing.T) { - if testing.Short() { - t.SkipNow() - } - t.Parallel() - - for i := 0; i < 20; i++ { - go ExampleJsonTransaction(createTestTxObj(t), MAX_RETRIES, t) - time.Sleep(1 * time.Millisecond) - } - -} - -func TestParallelTx1(t *testing.T) { - if testing.Short() { - t.SkipNow() - } - t.Parallel() - ExampleJsonTransaction(createTestTxObj(t), MAX_RETRIES, t) - for i := 0; i < 100; i++ { - go ExampleJsonTransaction(createTestTxObj(t), MAX_RETRIES, t) - } -} - -func TestParallelTx2(t *testing.T) { - if testing.Short() { - t.SkipNow() - } - t.Parallel() - for i := 0; i < 100; i++ { - go ExampleJsonTransaction(createTestTxObj(t), MAX_RETRIES, t) - } - ExampleJsonTransaction(createTestTxObj(t), MAX_RETRIES, t) -} - -// Just in case the above parallel tests didn't fail, force a failure here -func TestParallelInterrupted(t *testing.T) { - t.Parallel() - - testTx := createTestTxObj(t) - defer func() { - if err := recover(); err != nil { - t.Errorf("initiateRead() failed in parallel %s", err) - } - }() - verifyErrNil(testTx.initiateRead(), t) - - _, err := testTx.Do("WATCH", "testKeyA") - verifyErrNil(err, t) - - testValueA, err := redis.String(testTx.Do("GET", "testKeyA")) - if err != nil { - if err == redis.ErrNil { - t.Log("redis.ErrNil") - } else { - t.Error(err) - } - } - - _, err = testTx.Do("WATCH", "testKeyB") - if err != nil { - if err == redis.ErrNil { - t.Log("redis.ErrNil") - } else { - t.Error(err) - } - } - - testValueB, err := redis.String(testTx.Do("GET", "testKeyB")) - if err != nil { - if err == redis.ErrNil { - t.Log("redis.ErrNil") - } else { - t.Error(err) - } - } - // Stand in for what real updates would do - testValueB = testValueB + "+updates" - testValueA = testValueA + "+updates" - - // Simulating another client interrupts transaction, causing exec to fail - ExampleJsonTransaction(createTestTxObj(t), MAX_RETRIES, t) - - verifyErrNil(testTx.initiateWrite(), t) - verifyErrNil(testTx.Send("SET", "testKeyA", testValueA), t) - verifyErrNil(testTx.Send("SET", "testKeyB", testValueB), t) - - keys, err := (testTx.Do("EXEC")) - // Expect error - if keys != nil { - t.Error("Keys not nil; exec should have been interrupted") - } - verifyErrNil(err, t) - - testTx.close() -} diff --git a/models/models.go b/models/models.go index abfeeb6..cc1def7 100644 --- a/models/models.go +++ b/models/models.go @@ -40,4 +40,5 @@ type User struct { DownMultiplier float64 `json:"down_multiplier"` Slots int64 `json:"slots"` SlotsUsed int64 `json:"slots_used"` + Snatches uint `json:"snatches"` } From 42f9427c014450ffa6793099052bec3698693dd5 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Sat, 7 Sep 2013 20:22:30 -0400 Subject: [PATCH 2/9] Changed peer schema and added additional tests --- cache/cache.go | 17 +-- cache/redis/redis.go | 222 ++++++++++++++++++++++---------- cache/redis/redis_bench_test.go | 133 +++++++++++++++++++ cache/redis/redis_test.go | 205 +++++++++-------------------- cache/redis/tx_test.go | 190 +++++++++++++++++++++++++++ server/announce.go | 6 - 6 files changed, 541 insertions(+), 232 deletions(-) create mode 100644 cache/redis/redis_bench_test.go create mode 100644 cache/redis/tx_test.go diff --git a/cache/cache.go b/cache/cache.go index e477ede..702ea49 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -7,7 +7,6 @@ package cache import ( - "errors" "fmt" "github.com/pushrax/chihaya/config" @@ -15,9 +14,7 @@ import ( ) var ( - drivers = make(map[string]Driver) - ErrTxDone = errors.New("cache: Transaction has already been committed or rolled back") - ErrTxConflict = errors.New("cache: Commit interrupted, update transaction and repeat") + drivers = make(map[string]Driver) ) type Driver interface { @@ -57,15 +54,9 @@ type Pool interface { Get() (Tx, error) } -// Tx represents an in-progress data store transaction. -// A transaction must end with a call to Commit or Rollback. -// -// After a call to Commit or Rollback, all operations on the -// transaction must fail with ErrTxDone. +// The transmit object is the interface to add, remove and modify +// data in the cache type Tx interface { - Commit() error - Rollback() error - // Reads FindUser(passkey string) (*models.User, bool, error) FindTorrent(infohash string) (*models.Torrent, bool, error) @@ -86,4 +77,6 @@ type Tx interface { RemoveTorrent(t *models.Torrent) error AddUser(u *models.User) error RemoveUser(u *models.User) error + WhitelistClient(peerID string) error + UnWhitelistClient(peerID string) error } diff --git a/cache/redis/redis.go b/cache/redis/redis.go index 7281f0f..3de4ee5 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -16,7 +16,6 @@ package redis import ( "errors" "strconv" - "strings" "time" "github.com/garyburd/redigo/redis" @@ -32,10 +31,11 @@ var ( ErrCreatePeer = errors.New("redis: Incorrect reply length for peer") ErrMarkActive = errors.New("redis: Torrent doesn't exist") - SeederSuffix = ":seeders" - LeecherSuffix = ":leechers" + SeederPrefix = "seeders:" + LeecherPrefix = "leechers:" TorrentPrefix = "torrent:" UserPrefix = "user:" + PeerPrefix = "peer:" ) type driver struct{} @@ -77,11 +77,15 @@ func (p *Pool) Close() error { } func (p *Pool) Get() (cache.Tx, error) { - return &Tx{ + retTx := &Tx{ conf: p.conf, done: false, Conn: p.pool.Get(), - }, nil + } + // Test valid connection before returning + _, err := retTx.Do("PING") + + return retTx, err } type Tx struct { @@ -99,37 +103,6 @@ func (tx *Tx) close() { tx.Conn.Close() } -func (tx *Tx) Commit() error { - if tx.done { - return cache.ErrTxDone - } - if tx.multi == true { - execResponse, err := tx.Do("EXEC") - if execResponse == nil { - tx.multi = false - return cache.ErrTxConflict - } - if err != nil { - return err - } - } - tx.close() - return nil -} - -func (tx *Tx) Rollback() error { - if tx.done { - return cache.ErrTxDone - } - // Undoes watches and multi - if _, err := tx.Do("DISCARD"); err != nil { - return err - } - tx.multi = false - tx.close() - return nil -} - func createUser(userVals []string) (*models.User, error) { if len(userVals) != 7 { return nil, ErrCreateUser @@ -163,7 +136,7 @@ func createUser(userVals []string) (*models.User, error) { return &models.User{ID, Passkey, UpMultiplier, DownMultiplier, Slots, SlotsUsed, uint(Snatches)}, nil } -func createTorrent(torrentVals []string, seeders map[string]models.Peer, leechers map[string]models.Peer) (*models.Torrent, error) { +func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { if len(torrentVals) != 7 { return nil, ErrCreateTorrent } @@ -192,14 +165,32 @@ func createTorrent(torrentVals []string, seeders map[string]models.Peer, leecher if err != nil { return nil, err } - return &models.Torrent{ID, Infohash, Active, seeders, leechers, uint(Snatches), UpMultiplier, DownMultiplier, LastAction}, nil + seeders, err := tx.getPeers(ID, SeederPrefix) + if err != nil { + return nil, err + } + leechers, err := tx.getPeers(ID, LeecherPrefix) + if err != nil { + return nil, err + } + return &models.Torrent{ID, Infohash, Active, seeders, leechers, uint(Snatches), UpMultiplier, DownMultiplier, LastAction}, nil } -// Prevents adding duplicate peers, and doesn't return error on dup add -func (tx *Tx) addPeer(infohash string, peer *models.Peer, suffix string) error { - setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix - _, err := tx.Do("SADD", setKey, *peer) +// hashkey relies on combination of peerID, userID, and torrentID being unique +func (tx *Tx) setPeer(peer *models.Peer, peerTypePrefix string) error { + hashKey := tx.conf.Prefix + peerTypePrefix + getPeerHashKey(peer) + _, err := tx.Do("HMSET", hashKey, + "id", peer.ID, + "user_id", peer.UserID, + "torrent_id", peer.TorrentID, + "ip", peer.IP, + "port", peer.Port, + "uploaded", peer.Uploaded, + "downloaded", peer.Downloaded, + "left", peer.Left, + "last_announce", peer.LastAnnounce) + if err != nil { return err } @@ -207,8 +198,8 @@ func (tx *Tx) addPeer(infohash string, peer *models.Peer, suffix string) error { } // Will not return an error if the peer doesn't exist -func (tx *Tx) removePeer(infohash string, peer *models.Peer, suffix string) error { - setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix +func (tx *Tx) removePeer(peer *models.Peer, peerTypePrefix string) error { + setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(peer.TorrentID, 36) _, err := tx.Do("SREM", setKey, *peer) if err != nil { return err @@ -216,21 +207,42 @@ func (tx *Tx) removePeer(infohash string, peer *models.Peer, suffix string) erro return nil } -func (tx *Tx) addPeers(infohash string, peers map[string]models.Peer, suffix string) error { - setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix +func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { - err := tx.Send("SADD", setKey, peer) + hashKey := tx.conf.Prefix + peerTypePrefix + getPeerHashKey(&peer) + _, err := tx.Do("DEL", hashKey) if err != nil { return err } + delete(peers, peer.ID) } - tx.Flush() - tx.Receive() + // Only delete the set if all the peer deletions were successful + setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) + _, err := tx.Do("DEL", setKey) + if err != nil { + return err + } + return nil } -func createPeer(peerString string) (*models.Peer, error) { - peerVals := strings.Split(strings.Trim(peerString, "{}"), " ") +func getPeerHashKey(peer *models.Peer) string { + return peer.ID + ":" + strconv.FormatUint(peer.UserID, 36) + ":" + strconv.FormatUint(peer.TorrentID, 36) +} + +func (tx *Tx) addPeers(peers map[string]models.Peer, peerTypePrefix string) error { + for _, peer := range peers { + setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(peer.TorrentID, 36) + _, err := tx.Do("SADD", setKey, getPeerHashKey(&peer)) + if err != nil { + return err + } + tx.setPeer(&peer, peerTypePrefix) + } + return nil +} + +func createPeer(peerVals []string) (*models.Peer, error) { if len(peerVals) != 9 { return nil, ErrCreatePeer } @@ -269,12 +281,21 @@ func createPeer(peerString string) (*models.Peer, error) { } -func (tx *Tx) getPeers(infohash string, suffix string) (peers map[string]models.Peer, err error) { +func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[string]models.Peer, err error) { peers = make(map[string]models.Peer) - setKey := tx.conf.Prefix + TorrentPrefix + infohash + suffix + setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) peerStrings, err := redis.Strings(tx.Do("SMEMBERS", setKey)) - for peerIndex := range peerStrings { - peer, err := createPeer(peerStrings[peerIndex]) + if err != nil { + return peers, err + } + // Keys map to peer objects stored in hashes + for _, peerHashKey := range peerStrings { + hashKey := tx.conf.Prefix + peerTypePrefix + peerHashKey + peerVals, err := redis.Strings(tx.Do("HVALS", hashKey)) + if err != nil { + return peers, err + } + peer, err := createPeer(peerVals) if err != nil { return nil, err } @@ -297,18 +318,28 @@ func (tx *Tx) AddTorrent(t *models.Torrent) error { return err } - tx.addPeers(t.Infohash, t.Seeders, SeederSuffix) - tx.addPeers(t.Infohash, t.Leechers, LeecherSuffix) + tx.addPeers(t.Seeders, SeederPrefix) + tx.addPeers(t.Leechers, LeecherPrefix) return nil } func (tx *Tx) RemoveTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash - _, err := tx.Do("HDEL", hashkey) + _, err := tx.Do("DEL", hashkey) if err != nil { return err } + // Remove seeders and leechers as well + err = tx.removePeers(t.ID, t.Seeders, SeederPrefix) + if err != nil { + return err + } + err = tx.removePeers(t.ID, t.Leechers, LeecherPrefix) + if err != nil { + return err + } + return nil } @@ -330,7 +361,7 @@ func (tx *Tx) AddUser(u *models.User) error { func (tx *Tx) RemoveUser(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey - _, err := tx.Do("HDEL", hashkey) + _, err := tx.Do("DEL", hashkey) if err != nil { return err } @@ -352,6 +383,7 @@ func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { return foundUser, true, nil } +// This is a mulple action command, it's not internally atomic func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { hashkey := tx.conf.Prefix + TorrentPrefix + infohash torrentStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) @@ -361,9 +393,7 @@ func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { return nil, false, nil } - seeders, err := tx.getPeers(infohash, SeederSuffix) - leechers, err := tx.getPeers(infohash, LeecherSuffix) - foundTorrent, err := createTorrent(torrentStrings, seeders, leechers) + foundTorrent, err := tx.createTorrent(torrentStrings) if err != nil { return nil, false, err } @@ -372,7 +402,19 @@ func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { func (tx *Tx) ClientWhitelisted(peerID string) (exists bool, err error) { key := tx.conf.Prefix + "whitelist" - return redis.Bool(tx.Do("ISMEMBER", key, peerID)) + return redis.Bool(tx.Do("SISMEMBER", key, peerID)) +} + +func (tx *Tx) WhitelistClient(peerID string) error { + key := tx.conf.Prefix + "whitelist" + _, err := tx.Do("SADD", key, peerID) + return err +} + +func (tx *Tx) UnWhitelistClient(peerID string) error { + key := tx.conf.Prefix + "whitelist" + _, err := tx.Do("SREM", key, peerID) + return err } // This is a mulple action command, it's not internally atomic @@ -407,28 +449,66 @@ func (tx *Tx) MarkActive(torrent *models.Torrent) error { return nil } -func (tx *Tx) AddLeecher(t *models.Torrent, p *models.Peer) error { - return tx.addPeer(t.Infohash, p, LeecherSuffix) +func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { + setKey := tx.conf.Prefix + LeecherPrefix + strconv.FormatUint(torrent.ID, 36) + _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) + if err != nil { + return err + } + err = tx.setPeer(peer, LeecherPrefix) + if err != nil { + return err + } + if torrent.Leechers == nil { + torrent.Leechers = make(map[string]models.Peer) + } + torrent.Leechers[peer.ID] = *peer + return nil } +// Setting assumes it is already a leecher, and just needs to be updated +// Maybe eventually there will be a move from leecher to seeder method func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { - return tx.addPeer(t.Infohash, p, LeecherSuffix) + return tx.setPeer(p, LeecherPrefix) } func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { - return tx.removePeer(t.Infohash, p, LeecherSuffix) + err := tx.removePeer(p, LeecherPrefix) + if err != nil { + return err + } + delete(t.Leechers, p.ID) + return nil } -func (tx *Tx) AddSeeder(t *models.Torrent, p *models.Peer) error { - return tx.addPeer(t.Infohash, p, SeederSuffix) +func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { + setKey := tx.conf.Prefix + SeederPrefix + strconv.FormatUint(torrent.ID, 36) + _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) + if err != nil { + return err + } + err = tx.setPeer(peer, SeederPrefix) + if err != nil { + return err + } + if torrent.Seeders == nil { + torrent.Seeders = make(map[string]models.Peer) + } + torrent.Seeders[peer.ID] = *peer + return nil } func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { - return tx.addPeer(t.Infohash, p, SeederSuffix) + return tx.setPeer(p, SeederPrefix) } func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { - return tx.removePeer(t.Infohash, p, SeederSuffix) + err := tx.removePeer(p, SeederPrefix) + if err != nil { + return err + } + delete(t.Seeders, p.ID) + return nil } func (tx *Tx) IncrementSlots(u *models.User) error { diff --git a/cache/redis/redis_bench_test.go b/cache/redis/redis_bench_test.go new file mode 100644 index 0000000..7c14bee --- /dev/null +++ b/cache/redis/redis_bench_test.go @@ -0,0 +1,133 @@ +// Copyright 2013 The Chihaya Authors. All rights reserved. +// Use of this source code is governed by the BSD 2-Clause license, +// which can be found in the LICENSE file. + +// Benchmarks two different redis schemeas +package redis + +import ( + "errors" + "testing" + + "github.com/garyburd/redigo/redis" + + "github.com/pushrax/chihaya/models" +) + +var ( + ErrTxDone = errors.New("cache: Transaction has already been committed or rolled back") + ErrTxConflict = errors.New("cache: Commit interrupted, update transaction and repeat") +) + +// Maximum number of parallel retries; depends on system latency +const MAX_RETRIES = 9000 + +// Legacy JSON support for benching +func (tx *Tx) initiateWrite() error { + if tx.done { + return ErrTxDone + } + if tx.multi != true { + tx.multi = true + return tx.Send("MULTI") + } + return nil +} + +func (tx *Tx) initiateRead() error { + if tx.done { + return ErrTxDone + } + if tx.multi == true { + panic("Tried to read during MULTI") + } + return nil +} + +func (tx *Tx) Commit() error { + if tx.done { + return ErrTxDone + } + if tx.multi == true { + execResponse, err := tx.Do("EXEC") + if execResponse == nil { + tx.multi = false + return ErrTxConflict + } + if err != nil { + return err + } + } + tx.close() + return nil +} + +func (tx *Tx) Rollback() error { + if tx.done { + return ErrTxDone + } + // Undoes watches and multi + if _, err := tx.Do("DISCARD"); err != nil { + return err + } + tx.multi = false + tx.close() + return nil +} + +func ExampleRedisTypesSchemaFindUser(passkey string, t TestReporter) (*models.User, bool) { + testTx := createTestTxObj(t) + hashkey := testTx.conf.Prefix + UserPrefix + passkey + userVals, err := redis.Strings(testTx.Do("HVALS", hashkey)) + if len(userVals) == 0 { + return nil, false + } + verifyErrNil(err, t) + compareUser, err := createUser(userVals) + verifyErrNil(err, t) + return compareUser, true +} + +func BenchmarkRedisTypesSchemaRemoveSeeder(b *testing.B) { + for bCount := 0; bCount < b.N; bCount++ { + //TODO this needs to be updated + b.Error("Unimplemented") + + } +} + +func BenchmarkRedisTypesSchemaFindUser(b *testing.B) { + + // Ensure successful user find ( a failed lookup may have different performance ) + b.StopTimer() + testUser := createTestUser() + testTx := createTestTxObj(b) + hashkey := testTx.conf.Prefix + UserPrefix + testUser.Passkey + reply, err := testTx.Do("HMSET", hashkey, + "id", testUser.ID, + "passkey", testUser.Passkey, + "up_multiplier", testUser.UpMultiplier, + "down_multiplier", testUser.DownMultiplier, + "slots", testUser.Slots, + "slots_used", testUser.SlotsUsed) + + if reply == nil { + b.Log("no hash fields added!") + } + verifyErrNil(err, b) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + + compareUser, exists := ExampleRedisTypesSchemaFindUser(testUser.Passkey, b) + + b.StopTimer() + if !exists { + b.Error("User not found!") + } + if testUser != *compareUser { + b.Errorf("user mismatch: %v vs. %v", compareUser, testUser) + } + b.StartTimer() + } +} diff --git a/cache/redis/redis_test.go b/cache/redis/redis_test.go index 3a18ea1..144a7f3 100644 --- a/cache/redis/redis_test.go +++ b/cache/redis/redis_test.go @@ -5,21 +5,56 @@ package redis import ( + "crypto/rand" + "io" "os" + "strconv" "testing" "github.com/garyburd/redigo/redis" - "github.com/pushrax/chihaya/cache" "github.com/pushrax/chihaya/config" "github.com/pushrax/chihaya/models" ) -// Maximum number of parallel retries; depends on system latency -const MAX_RETRIES = 9000 +var ( + testTorrentIDCounter uint64 + testUserIDCounter uint64 + testPeerIDCounter int +) -const sample_infohash = "58c290f4ea1efb3adcb8c1ed2643232117577bcd" -const sample_passkey = "32426b162be0bce5428e7e36afaf734ae5afb355" +func createTestTorrentID() uint64 { + testTorrentIDCounter++ + return testTorrentIDCounter +} + +func createTestUserID() uint64 { + testUserIDCounter++ + return testUserIDCounter +} + +func createTestPeerID() string { + testPeerIDCounter++ + return "-testPeerID-" + strconv.Itoa(testPeerIDCounter) +} + +func createTestInfohash() string { + uuid := make([]byte, 40) + n, err := io.ReadFull(rand.Reader, uuid) + if n != len(uuid) || err != nil { + panic(err) + } + return string(uuid) +} + +func createTestPasskey() string { + uuid := make([]byte, 40) + n, err := io.ReadFull(rand.Reader, uuid) + if n != len(uuid) || err != nil { + panic(err) + } + return string(uuid) +} // Common interface for benchmarks and test error reporting type TestReporter interface { @@ -35,28 +70,6 @@ func verifyErrNil(err error, t TestReporter) { } } -// Legacy JSON support for benching -func (tx *Tx) initiateWrite() error { - if tx.done { - return cache.ErrTxDone - } - if tx.multi != true { - tx.multi = true - return tx.Send("MULTI") - } - return nil -} - -func (tx *Tx) initiateRead() error { - if tx.done { - return cache.ErrTxDone - } - if tx.multi == true { - panic("Tried to read during MULTI") - } - return nil -} - func createTestTxObj(t TestReporter) *Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) conf := &testConfig.Cache @@ -72,8 +85,6 @@ func createTestTxObj(t TestReporter) *Tx { }, } - //testDialFunc := makeDialFunc(&testConfig.Cache) - //testConn, err := testDialFunc() txObj := &Tx{ conf: testPool.conf, done: false, @@ -83,85 +94,53 @@ func createTestTxObj(t TestReporter) *Tx { verifyErrNil(err, t) // Test connection before returning - //txObj := Tx{&testConfig.Cache, false, false, testConn} _, err = txObj.Do("PING") verifyErrNil(err, t) return txObj } func createTestUser() models.User { - testUser := models.User{214, "32426b162be0bce5428e7e36afaf734ae5afb355", 1.01, 1.0, 4, 2, 7} + testUser := models.User{createTestUserID(), createTestPasskey(), 1.01, 1.0, 4, 2, 7} return testUser } -func createSeeders() []models.Peer { - testSeeders := make([]models.Peer, 4) - testSeeders[0] = models.Peer{"testPeerID0", 57005, 48879, "testIP", 6889, 1024, 3000, 4200, 6} - testSeeders[1] = models.Peer{"testPeerID1", 10101, 48879, "testIP", 6889, 1024, 3000, 4200, 6} - testSeeders[2] = models.Peer{"testPeerID2", 29890, 48879, "testIP", 6889, 1024, 3000, 4200, 6} - testSeeders[3] = models.Peer{"testPeerID3", 65261, 48879, "testIP", 6889, 1024, 3000, 4200, 6} - return testSeeders +func createTestPeer(userID uint64, torrentID uint64) models.Peer { + + return models.Peer{createTestPeerID(), userID, torrentID, "127.0.0.1", 6889, 1024, 3000, 4200, 11} } -func createLeechers() []models.Peer { - testLeechers := make([]models.Peer, 1) - testLeechers[0] = models.Peer{"testPeerID", 11111, 48879, "testIP", 6889, 1024, 3000, 4200, 6} - return testLeechers +func createTestPeers(torrentID uint64, num int) map[string]models.Peer { + testPeers := make(map[string]models.Peer) + for i := 0; i < num; i++ { + tempPeer := createTestPeer(createTestUserID(), torrentID) + testPeers[tempPeer.ID] = tempPeer + } + return testPeers } func createTestTorrent() models.Torrent { - testSeeders := createSeeders() - testLeechers := createLeechers() + torrentInfohash := createTestInfohash() + torrentID := createTestTorrentID() - seeders := make(map[string]models.Peer) - for i := range testSeeders { - seeders[testSeeders[i].ID] = testSeeders[i] - } + testSeeders := createTestPeers(torrentID, 4) + testLeechers := createTestPeers(torrentID, 2) - leechers := make(map[string]models.Peer) - for i := range testLeechers { - leechers[testLeechers[i].ID] = testLeechers[i] - } - - testTorrent := models.Torrent{48879, sample_infohash, true, seeders, leechers, 11, 0.0, 0.0, 0} + testTorrent := models.Torrent{torrentID, torrentInfohash, true, testSeeders, testLeechers, 11, 0.0, 0.0, 0} return testTorrent } -func ExampleRedisTypeSchemaRemoveSeeder(torrent *models.Torrent, peer *models.Peer, t TestReporter) { - testTx := createTestTxObj(t) - setkey := testTx.conf.Prefix + "torrent:" + torrent.Infohash + ":seeders" - reply, err := redis.Int(testTx.Do("SREM", setkey, *peer)) - if reply == 0 { - t.Errorf("remove %v failed", *peer) - } - verifyErrNil(err, t) -} - -func ExampleRedisTypesSchemaFindUser(passkey string, t TestReporter) (*models.User, bool) { - testTx := createTestTxObj(t) - hashkey := testTx.conf.Prefix + UserPrefix + passkey - userVals, err := redis.Strings(testTx.Do("HVALS", hashkey)) - if len(userVals) == 0 { - return nil, false - } - verifyErrNil(err, t) - compareUser, err := createUser(userVals) - verifyErrNil(err, t) - return compareUser, true -} - func TestFindUserSuccess(t *testing.T) { testUser := createTestUser() testTx := createTestTxObj(t) - hashkey := testTx.conf.Prefix + UserPrefix + sample_passkey + hashkey := testTx.conf.Prefix + UserPrefix + testUser.Passkey _, err := testTx.Do("DEL", hashkey) verifyErrNil(err, t) err = testTx.AddUser(&testUser) verifyErrNil(err, t) - compareUser, exists := ExampleRedisTypesSchemaFindUser(sample_passkey, t) + compareUser, exists := ExampleRedisTypesSchemaFindUser(testUser.Passkey, t) if !exists { t.Error("User not found!") @@ -183,11 +162,11 @@ func TestAddGetPeers(t *testing.T) { testTx := createTestTxObj(t) testTorrent := createTestTorrent() - setkey := testTx.conf.Prefix + "torrent:" + testTorrent.Infohash + ":seeders" + setkey := testTx.conf.Prefix + SeederPrefix + strconv.FormatUint(testTorrent.ID, 36) testTx.Do("DEL", setkey) - testTx.addPeers(testTorrent.Infohash, testTorrent.Seeders, ":seeders") - peerMap, err := testTx.getPeers(sample_infohash, ":seeders") + testTx.addPeers(testTorrent.Seeders, SeederPrefix) + peerMap, err := testTx.getPeers(testTorrent.ID, SeederPrefix) if err != nil { t.Error(err) } else if len(peerMap) != len(testTorrent.Seeders) { @@ -195,67 +174,7 @@ func TestAddGetPeers(t *testing.T) { } } -func BenchmarkRedisTypesSchemaRemoveSeeder(b *testing.B) { - for bCount := 0; bCount < b.N; bCount++ { - // Ensure that remove completes successfully, - // even if it doesn't impact the performance - b.StopTimer() - testTx := createTestTxObj(b) - testTorrent := createTestTorrent() - setkey := testTx.conf.Prefix + "torrent:" + testTorrent.Infohash + ":seeders" - testSeeders := createSeeders() - reply, err := redis.Int(testTx.Do("SADD", setkey, - testSeeders[0], - testSeeders[1], - testSeeders[2], - testSeeders[3])) - - if reply == 0 { - b.Log("no keys added!") - } - verifyErrNil(err, b) - b.StartTimer() - - ExampleRedisTypeSchemaRemoveSeeder(&testTorrent, &testSeeders[2], b) - } -} - -func BenchmarkRedisTypesSchemaFindUser(b *testing.B) { - - // Ensure successful user find ( a failed lookup may have different performance ) - b.StopTimer() - testUser := createTestUser() - testTx := createTestTxObj(b) - hashkey := testTx.conf.Prefix + UserPrefix + sample_passkey - reply, err := testTx.Do("HMSET", hashkey, - "id", testUser.ID, - "passkey", testUser.Passkey, - "up_multiplier", testUser.UpMultiplier, - "down_multiplier", testUser.DownMultiplier, - "slots", testUser.Slots, - "slots_used", testUser.SlotsUsed) - - if reply == nil { - b.Log("no hash fields added!") - } - verifyErrNil(err, b) - b.StartTimer() - - for bCount := 0; bCount < b.N; bCount++ { - - compareUser, exists := ExampleRedisTypesSchemaFindUser(sample_passkey, b) - - b.StopTimer() - if !exists { - b.Error("User not found!") - } - if testUser != *compareUser { - b.Errorf("user mismatch: %v vs. %v", compareUser, testUser) - } - b.StartTimer() - } -} - +// Legacy tests func TestReadAfterWrite(t *testing.T) { // Test requires panic defer func() { diff --git a/cache/redis/tx_test.go b/cache/redis/tx_test.go new file mode 100644 index 0000000..a82b771 --- /dev/null +++ b/cache/redis/tx_test.go @@ -0,0 +1,190 @@ +// Copyright 2013 The Chihaya Authors. All rights reserved. +// Use of this source code is governed by the BSD 2-Clause license, +// which can be found in the LICENSE file. + +package redis + +import ( + "fmt" + "math/rand" + "os" + "testing" + "time" + + "github.com/pushrax/chihaya/cache" + "github.com/pushrax/chihaya/config" +) + +func panicErrNil(err error) { + if err != nil { + fmt.Println(err) + panic(err) + } +} + +func createTestTx() cache.Tx { + testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) + panicErrNil(err) + conf := &testConfig.Cache + + testPool, err := cache.Open(conf) + panicErrNil(err) + + txObj, err := testPool.Get() + panicErrNil(err) + + return txObj +} + +func TestUser(t *testing.T) { + tx := createTestTx() + testUser1 := createTestUser() + testUser2 := createTestUser() + + panicErrNil(tx.AddUser(&testUser1)) + foundUser, found, err := tx.FindUser(testUser1.Passkey) + panicErrNil(err) + if !found { + t.Error("user not found") + } + if *foundUser != testUser1 { + t.Error("found user mismatch") + } + + foundUser, found, err = tx.FindUser(testUser2.Passkey) + panicErrNil(err) + if found { + t.Error("user found") + } + + err = tx.RemoveUser(&testUser1) + panicErrNil(err) + foundUser, found, err = tx.FindUser(testUser1.Passkey) + panicErrNil(err) + if found { + t.Error("removed user found") + } +} + +func TestTorrent(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + testTorrent2 := createTestTorrent() + + panicErrNil(tx.AddTorrent(&testTorrent1)) + foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + if !found { + t.Error("torrent not found") + } + // Incomplete comparison as maps cannot be compared + if foundTorrent.Infohash != testTorrent1.Infohash { + t.Error("found torrent mismatch") + } + foundTorrent, found, err = tx.FindTorrent(testTorrent2.Infohash) + panicErrNil(err) + if found { + t.Error("torrent found") + } + + panicErrNil(tx.RemoveTorrent(&testTorrent1)) + foundTorrent, found, err = tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + if found { + t.Error("removed torrent found") + } +} +func TestClient(t *testing.T) { + tx := createTestTx() + testPeerID1 := "-lt0D30-" + testPeerID2 := "TIX0192" + + panicErrNil(tx.WhitelistClient(testPeerID1)) + found, err := tx.ClientWhitelisted(testPeerID1) + panicErrNil(err) + if !found { + t.Error("peerID not found") + } + + found, err = tx.ClientWhitelisted(testPeerID2) + panicErrNil(err) + if found { + t.Error("peerID found") + } + + panicErrNil(tx.UnWhitelistClient(testPeerID1)) + found, err = tx.ClientWhitelisted(testPeerID1) + panicErrNil(err) + if found { + t.Error("removed peerID found") + } +} + +func TestPeers(t *testing.T) { + tx := createTestTx() + + // Randomly generated strings would be safter to test with + testTorrent1 := createTestTorrent() + testTorrent2 := createTestTorrent() + foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + if found { + testTorrent1 = *foundTorrent + } else { + panicErrNil(tx.AddTorrent(&testTorrent1)) + } + foundTorrent, found, err = tx.FindTorrent(testTorrent2.Infohash) + panicErrNil(err) + if found { + testTorrent2 = *foundTorrent + } else { + panicErrNil(tx.AddTorrent(&testTorrent2)) + } + + testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) + testSeeder2 := createTestPeer(createTestUserID(), testTorrent2.ID) + if testSeeder1 == testSeeder2 { + t.Error("seeders should not be equal") + } + + if _, exists := testTorrent1.Seeders[testSeeder1.ID]; exists { + t.Log("seeder aleady exists, removing") + err := tx.RemoveSeeder(&testTorrent1, &testSeeder1) + if err != nil { + t.Error(err) + } + if _, exists := testTorrent1.Seeders[testSeeder1.ID]; exists { + t.Error("Remove seeder failed") + } + } + + panicErrNil(tx.AddSeeder(&testTorrent1, &testSeeder1)) + if seeder1, exists := testTorrent1.Seeders[testSeeder1.ID]; !exists { + t.Error("seeder not added locally") + } else if seeder1 != testSeeder1 { + t.Error("seeder changed") + } + foundTorrent, found, err = tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + if !found { + t.Error("torrent should exist") + } + if seeder1, exists := foundTorrent.Seeders[testSeeder1.ID]; !exists { + t.Error("seeder not added") + } else if seeder1 != testSeeder1 { + t.Error("seeder changed") + } + + // Update a seeder, set it, then check to make sure it updated + r := rand.New(rand.NewSource(time.Now().UnixNano())) + testSeeder1.Downloaded += uint64(r.Int63()) + panicErrNil(tx.SetSeeder(&testTorrent1, &testSeeder1)) + foundTorrent, found, err = tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + if seeder1, exists := foundTorrent.Seeders[testSeeder1.ID]; !exists { + t.Error("seeder not added") + } else if seeder1 != testSeeder1 { + t.Errorf("seeder changed from %v to %v", testSeeder1, seeder1) + } + +} diff --git a/server/announce.go b/server/announce.go index 1d7a142..150ca19 100644 --- a/server/announce.go +++ b/server/announce.go @@ -197,12 +197,6 @@ func (s Server) serveAnnounce(w http.ResponseWriter, r *http.Request) { peer.IP = ip } - // If the transaction failed, retry - err = tx.Commit() - if err != nil { - continue - } - // Generate the response seedCount := len(torrent.Seeders) leechCount := len(torrent.Leechers) From 3caa06b5f6bb3e2c780763c9e8d60b5792f14916 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Tue, 10 Sep 2013 01:02:40 -0400 Subject: [PATCH 3/9] Updated tests, added LeecherFinished to cache.Tx --- cache/cache.go | 3 + cache/redis/redis.go | 77 +++++++-- cache/redis/redis_test.go | 51 +----- cache/redis/tx_test.go | 344 +++++++++++++++++++++++++++----------- server/announce.go | 12 +- 5 files changed, 313 insertions(+), 174 deletions(-) diff --git a/cache/cache.go b/cache/cache.go index 702ea49..bcd2a4d 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -73,6 +73,9 @@ type Tx interface { SetSeeder(t *models.Torrent, p *models.Peer) error IncrementSlots(u *models.User) error DecrementSlots(u *models.User) error + LeecherFinished(t *models.Torrent, p *models.Peer) error + + // Priming / Testing AddTorrent(t *models.Torrent) error RemoveTorrent(t *models.Torrent) error AddUser(u *models.User) error diff --git a/cache/redis/redis.go b/cache/redis/redis.go index 3de4ee5..90e3800 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -136,6 +136,7 @@ func createUser(userVals []string) (*models.User, error) { return &models.User{ID, Passkey, UpMultiplier, DownMultiplier, Slots, SlotsUsed, uint(Snatches)}, nil } +// This is a mulple action command, it's not internally atomic func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { if len(torrentVals) != 7 { return nil, ErrCreateTorrent @@ -177,9 +178,9 @@ func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { return &models.Torrent{ID, Infohash, Active, seeders, leechers, uint(Snatches), UpMultiplier, DownMultiplier, LastAction}, nil } -// hashkey relies on combination of peerID, userID, and torrentID being unique -func (tx *Tx) setPeer(peer *models.Peer, peerTypePrefix string) error { - hashKey := tx.conf.Prefix + peerTypePrefix + getPeerHashKey(peer) +// The peer hashkey relies on the combination of peerID, userID, and torrentID being unique +func (tx *Tx) setPeer(peer *models.Peer) error { + hashKey := tx.conf.Prefix + getPeerHashKey(peer) _, err := tx.Do("HMSET", hashKey, "id", peer.ID, "user_id", peer.UserID, @@ -199,17 +200,20 @@ func (tx *Tx) setPeer(peer *models.Peer, peerTypePrefix string) error { // Will not return an error if the peer doesn't exist func (tx *Tx) removePeer(peer *models.Peer, peerTypePrefix string) error { - setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(peer.TorrentID, 36) - _, err := tx.Do("SREM", setKey, *peer) + setKey := tx.conf.Prefix + getPeerSetKey(peerTypePrefix, peer) + _, err := tx.Do("SREM", setKey, getPeerHashKey(peer)) if err != nil { return err } + hashKey := tx.conf.Prefix + getPeerHashKey(peer) + _, err = tx.Do("DEL", hashKey) return nil } +// This is a mulple action command, it's not internally atomic func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { - hashKey := tx.conf.Prefix + peerTypePrefix + getPeerHashKey(&peer) + hashKey := tx.conf.Prefix + getPeerHashKey(&peer) _, err := tx.Do("DEL", hashKey) if err != nil { return err @@ -230,14 +234,19 @@ func getPeerHashKey(peer *models.Peer) string { return peer.ID + ":" + strconv.FormatUint(peer.UserID, 36) + ":" + strconv.FormatUint(peer.TorrentID, 36) } +func getPeerSetKey(prefix string, peer *models.Peer) string { + return prefix + strconv.FormatUint(peer.TorrentID, 36) +} + +// This is a mulple action command, it's not internally atomic func (tx *Tx) addPeers(peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { - setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(peer.TorrentID, 36) + setKey := tx.conf.Prefix + getPeerSetKey(peerTypePrefix, &peer) _, err := tx.Do("SADD", setKey, getPeerHashKey(&peer)) if err != nil { return err } - tx.setPeer(&peer, peerTypePrefix) + tx.setPeer(&peer) } return nil } @@ -281,6 +290,7 @@ func createPeer(peerVals []string) (*models.Peer, error) { } +// This is a mulple action command, it's not internally atomic func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[string]models.Peer, err error) { peers = make(map[string]models.Peer) setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) @@ -290,7 +300,7 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin } // Keys map to peer objects stored in hashes for _, peerHashKey := range peerStrings { - hashKey := tx.conf.Prefix + peerTypePrefix + peerHashKey + hashKey := tx.conf.Prefix + peerHashKey peerVals, err := redis.Strings(tx.Do("HVALS", hashKey)) if err != nil { return peers, err @@ -304,6 +314,7 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin return } +// This is a mulple action command, it's not internally atomic func (tx *Tx) AddTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("HMSET", hashkey, @@ -318,12 +329,18 @@ func (tx *Tx) AddTorrent(t *models.Torrent) error { return err } - tx.addPeers(t.Seeders, SeederPrefix) - tx.addPeers(t.Leechers, LeecherPrefix) - + err = tx.addPeers(t.Seeders, SeederPrefix) + if err != nil { + return err + } + err = tx.addPeers(t.Leechers, LeecherPrefix) + if err != nil { + return err + } return nil } +// This is a mulple action command, it's not internally atomic func (tx *Tx) RemoveTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("DEL", hashkey) @@ -449,13 +466,14 @@ func (tx *Tx) MarkActive(torrent *models.Torrent) error { return nil } +// This is a mulple action command, it's not internally atomic func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { setKey := tx.conf.Prefix + LeecherPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) if err != nil { return err } - err = tx.setPeer(peer, LeecherPrefix) + err = tx.setPeer(peer) if err != nil { return err } @@ -469,7 +487,12 @@ func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { // Setting assumes it is already a leecher, and just needs to be updated // Maybe eventually there will be a move from leecher to seeder method func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { - return tx.setPeer(p, LeecherPrefix) + err := tx.setPeer(p) + if err != nil { + return err + } + t.Leechers[p.ID] = *p + return nil } func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { @@ -481,13 +504,30 @@ func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { return nil } +func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error { + torrentIdKey := strconv.FormatUint(torrent.ID, 36) + seederSetKey := tx.conf.Prefix + SeederPrefix + torrentIdKey + leecherSetKey := tx.conf.Prefix + LeecherPrefix + torrentIdKey + + _, err := tx.Do("SMOVE", leecherSetKey, seederSetKey, getPeerHashKey(peer)) + if err != nil { + return err + } + torrent.Seeders[peer.ID] = *peer + delete(torrent.Leechers, peer.ID) + + err = tx.setPeer(peer) + return err +} + +// This is a mulple action command, it's not internally atomic func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { setKey := tx.conf.Prefix + SeederPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) if err != nil { return err } - err = tx.setPeer(peer, SeederPrefix) + err = tx.setPeer(peer) if err != nil { return err } @@ -499,7 +539,12 @@ func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { } func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { - return tx.setPeer(p, SeederPrefix) + err := tx.setPeer(p) + if err != nil { + return err + } + t.Seeders[p.ID] = *p + return nil } func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { diff --git a/cache/redis/redis_test.go b/cache/redis/redis_test.go index 144a7f3..777cfa8 100644 --- a/cache/redis/redis_test.go +++ b/cache/redis/redis_test.go @@ -104,21 +104,21 @@ func createTestUser() models.User { return testUser } -func createTestPeer(userID uint64, torrentID uint64) models.Peer { +func createTestPeer(userID uint64, torrentID uint64) *models.Peer { - return models.Peer{createTestPeerID(), userID, torrentID, "127.0.0.1", 6889, 1024, 3000, 4200, 11} + return &models.Peer{createTestPeerID(), userID, torrentID, "127.0.0.1", 6889, 1024, 3000, 4200, 11} } func createTestPeers(torrentID uint64, num int) map[string]models.Peer { testPeers := make(map[string]models.Peer) for i := 0; i < num; i++ { tempPeer := createTestPeer(createTestUserID(), torrentID) - testPeers[tempPeer.ID] = tempPeer + testPeers[tempPeer.ID] = *tempPeer } return testPeers } -func createTestTorrent() models.Torrent { +func createTestTorrent() *models.Torrent { torrentInfohash := createTestInfohash() torrentID := createTestTorrentID() @@ -127,34 +127,7 @@ func createTestTorrent() models.Torrent { testLeechers := createTestPeers(torrentID, 2) testTorrent := models.Torrent{torrentID, torrentInfohash, true, testSeeders, testLeechers, 11, 0.0, 0.0, 0} - return testTorrent -} - -func TestFindUserSuccess(t *testing.T) { - testUser := createTestUser() - testTx := createTestTxObj(t) - hashkey := testTx.conf.Prefix + UserPrefix + testUser.Passkey - _, err := testTx.Do("DEL", hashkey) - verifyErrNil(err, t) - - err = testTx.AddUser(&testUser) - verifyErrNil(err, t) - - compareUser, exists := ExampleRedisTypesSchemaFindUser(testUser.Passkey, t) - - if !exists { - t.Error("User not found!") - } - if testUser != *compareUser { - t.Errorf("user mismatch: %v vs. %v", compareUser, testUser) - } -} - -func TestFindUserFail(t *testing.T) { - compareUser, exists := ExampleRedisTypesSchemaFindUser("not_a_user_passkey", t) - if exists { - t.Errorf("User %v found when none should exist!", compareUser) - } + return &testTorrent } func TestAddGetPeers(t *testing.T) { @@ -174,20 +147,6 @@ func TestAddGetPeers(t *testing.T) { } } -// Legacy tests -func TestReadAfterWrite(t *testing.T) { - // Test requires panic - defer func() { - if err := recover(); err == nil { - t.Error("Read after write did not panic") - } - }() - - testTx := createTestTxObj(t) - verifyErrNil(testTx.initiateWrite(), t) - verifyErrNil(testTx.initiateRead(), t) -} - func TestCloseClosedTransaction(t *testing.T) { //require panic defer func() { diff --git a/cache/redis/tx_test.go b/cache/redis/tx_test.go index a82b771..f5d50c9 100644 --- a/cache/redis/tx_test.go +++ b/cache/redis/tx_test.go @@ -36,155 +36,295 @@ func createTestTx() cache.Tx { return txObj } -func TestUser(t *testing.T) { +func TestFindUserSuccess(t *testing.T) { tx := createTestTx() testUser1 := createTestUser() - testUser2 := createTestUser() panicErrNil(tx.AddUser(&testUser1)) foundUser, found, err := tx.FindUser(testUser1.Passkey) panicErrNil(err) if !found { - t.Error("user not found") + t.Error("user not found", testUser1) } if *foundUser != testUser1 { - t.Error("found user mismatch") - } - - foundUser, found, err = tx.FindUser(testUser2.Passkey) - panicErrNil(err) - if found { - t.Error("user found") - } - - err = tx.RemoveUser(&testUser1) - panicErrNil(err) - foundUser, found, err = tx.FindUser(testUser1.Passkey) - panicErrNil(err) - if found { - t.Error("removed user found") + t.Error("found user mismatch", *foundUser, testUser1) } } -func TestTorrent(t *testing.T) { +func TestFindUserFail(t *testing.T) { + tx := createTestTx() + testUser2 := createTestUser() + + foundUser, found, err := tx.FindUser(testUser2.Passkey) + panicErrNil(err) + if found { + t.Error("user found", foundUser) + } +} + +func TestRemoveUser(t *testing.T) { + tx := createTestTx() + testUser1 := createTestUser() + + panicErrNil(tx.AddUser(&testUser1)) + err := tx.RemoveUser(&testUser1) + panicErrNil(err) + foundUser, found, err := tx.FindUser(testUser1.Passkey) + panicErrNil(err) + if found { + t.Error("removed user found", foundUser) + } +} + +func TestFindTorrent(t *testing.T) { tx := createTestTx() testTorrent1 := createTestTorrent() - testTorrent2 := createTestTorrent() - panicErrNil(tx.AddTorrent(&testTorrent1)) + panicErrNil(tx.AddTorrent(testTorrent1)) foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) panicErrNil(err) if !found { - t.Error("torrent not found") + t.Error("torrent not found", testTorrent1) } - // Incomplete comparison as maps cannot be compared + // Incomplete comparison as maps make struct not nativly comparable if foundTorrent.Infohash != testTorrent1.Infohash { - t.Error("found torrent mismatch") - } - foundTorrent, found, err = tx.FindTorrent(testTorrent2.Infohash) - panicErrNil(err) - if found { - t.Error("torrent found") - } - - panicErrNil(tx.RemoveTorrent(&testTorrent1)) - foundTorrent, found, err = tx.FindTorrent(testTorrent1.Infohash) - panicErrNil(err) - if found { - t.Error("removed torrent found") + t.Error("found torrent mismatch", foundTorrent, testTorrent1) } } -func TestClient(t *testing.T) { + +func TestFindTorrentFail(t *testing.T) { + tx := createTestTx() + testTorrent2 := createTestTorrent() + + foundTorrent, found, err := tx.FindTorrent(testTorrent2.Infohash) + panicErrNil(err) + if found { + t.Error("torrent found", foundTorrent) + } +} + +func TestRemoveTorrent(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) + + panicErrNil(tx.RemoveTorrent(testTorrent1)) + foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + if found { + t.Error("removed torrent found", foundTorrent) + } +} + +func TestClientWhitelistSuccess(t *testing.T) { tx := createTestTx() testPeerID1 := "-lt0D30-" - testPeerID2 := "TIX0192" panicErrNil(tx.WhitelistClient(testPeerID1)) found, err := tx.ClientWhitelisted(testPeerID1) panicErrNil(err) if !found { - t.Error("peerID not found") - } - - found, err = tx.ClientWhitelisted(testPeerID2) - panicErrNil(err) - if found { - t.Error("peerID found") - } - - panicErrNil(tx.UnWhitelistClient(testPeerID1)) - found, err = tx.ClientWhitelisted(testPeerID1) - panicErrNil(err) - if found { - t.Error("removed peerID found") + t.Error("peerID not found", testPeerID1) } } -func TestPeers(t *testing.T) { +func TestClientWhitelistFail(t *testing.T) { tx := createTestTx() + testPeerID2 := "TIX0192" - // Randomly generated strings would be safter to test with + found, err := tx.ClientWhitelisted(testPeerID2) + panicErrNil(err) + if found { + t.Error("peerID found", testPeerID2) + } + +} + +func TestClientWhitelistRemove(t *testing.T) { + tx := createTestTx() + testPeerID1 := "-lt0D30-" + panicErrNil(tx.WhitelistClient(testPeerID1)) + panicErrNil(tx.UnWhitelistClient(testPeerID1)) + + found, err := tx.ClientWhitelisted(testPeerID1) + panicErrNil(err) + if found { + t.Error("removed peerID found", testPeerID1) + } +} + +func TestAddSeeder(t *testing.T) { + tx := createTestTx() testTorrent1 := createTestTorrent() - testTorrent2 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) + testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) + + panicErrNil(tx.AddSeeder(testTorrent1, testSeeder1)) foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) panicErrNil(err) - if found { - testTorrent1 = *foundTorrent - } else { - panicErrNil(tx.AddTorrent(&testTorrent1)) + foundSeeder, found := foundTorrent.Seeders[testSeeder1.ID] + if found && foundSeeder != *testSeeder1 { + t.Error("seeder not added to cache", testSeeder1) } - foundTorrent, found, err = tx.FindTorrent(testTorrent2.Infohash) - panicErrNil(err) - if found { - testTorrent2 = *foundTorrent - } else { - panicErrNil(tx.AddTorrent(&testTorrent2)) + foundSeeder, found = testTorrent1.Seeders[testSeeder1.ID] + if found && foundSeeder != *testSeeder1 { + t.Error("seeder not added to local", testSeeder1) } +} +func TestAddLeecher(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) + testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) + + tx.AddLeecher(testTorrent1, testLeecher1) + foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + foundLeecher, found := foundTorrent.Leechers[testLeecher1.ID] + if found && foundLeecher != *testLeecher1 { + t.Error("leecher not added to cache", testLeecher1) + } + foundLeecher, found = testTorrent1.Leechers[testLeecher1.ID] + if found && foundLeecher != *testLeecher1 { + t.Error("leecher not added to local", testLeecher1) + } +} + +func TestRemoveSeeder(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) - testSeeder2 := createTestPeer(createTestUserID(), testTorrent2.ID) - if testSeeder1 == testSeeder2 { - t.Error("seeders should not be equal") + tx.AddSeeder(testTorrent1, testSeeder1) + + panicErrNil(tx.RemoveSeeder(testTorrent1, testSeeder1)) + foundSeeder, found := testTorrent1.Seeders[testSeeder1.ID] + if found || foundSeeder == *testSeeder1 { + t.Error("seeder not removed from local", foundSeeder) } - if _, exists := testTorrent1.Seeders[testSeeder1.ID]; exists { - t.Log("seeder aleady exists, removing") - err := tx.RemoveSeeder(&testTorrent1, &testSeeder1) - if err != nil { - t.Error(err) - } - if _, exists := testTorrent1.Seeders[testSeeder1.ID]; exists { - t.Error("Remove seeder failed") - } - } - - panicErrNil(tx.AddSeeder(&testTorrent1, &testSeeder1)) - if seeder1, exists := testTorrent1.Seeders[testSeeder1.ID]; !exists { - t.Error("seeder not added locally") - } else if seeder1 != testSeeder1 { - t.Error("seeder changed") - } - foundTorrent, found, err = tx.FindTorrent(testTorrent1.Infohash) + foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) panicErrNil(err) - if !found { - t.Error("torrent should exist") - } - if seeder1, exists := foundTorrent.Seeders[testSeeder1.ID]; !exists { - t.Error("seeder not added") - } else if seeder1 != testSeeder1 { - t.Error("seeder changed") + foundSeeder, found = foundTorrent.Seeders[testSeeder1.ID] + if found || foundSeeder == *testSeeder1 { + t.Error("seeder not removed from cache", foundSeeder) } +} +func TestRemoveLeecher(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) + testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) + tx.AddLeecher(testTorrent1, testLeecher1) + + tx.RemoveLeecher(testTorrent1, testLeecher1) + foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + foundLeecher, found := foundTorrent.Leechers[testLeecher1.ID] + if found || foundLeecher == *testLeecher1 { + t.Error("leecher not removed from cache", foundLeecher) + } + foundLeecher, found = testTorrent1.Leechers[testLeecher1.ID] + if found || foundLeecher == *testLeecher1 { + t.Error("leecher not removed from local", foundLeecher) + } +} + +func TestSetSeeder(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) + testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) + tx.AddSeeder(testTorrent1, testSeeder1) + + testSeeder1.Uploaded += 100 + + tx.SetSeeder(testTorrent1, testSeeder1) + foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + foundSeeder, _ := foundTorrent.Seeders[testSeeder1.ID] + if foundSeeder != *testSeeder1 { + t.Error("seeder not updated in cache", testSeeder1) + } + foundSeeder, _ = testTorrent1.Seeders[testSeeder1.ID] + if foundSeeder != *testSeeder1 { + t.Error("seeder not updated in local", testSeeder1) + } +} + +func TestSetLeecher(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) + testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) + tx.AddLeecher(testTorrent1, testLeecher1) + + testLeecher1.Uploaded += 100 + + tx.SetLeecher(testTorrent1, testLeecher1) + foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + foundLeecher, _ := foundTorrent.Leechers[testLeecher1.ID] + if foundLeecher != *testLeecher1 { + t.Error("leecher not updated in cache", testLeecher1) + } + foundLeecher, _ = testTorrent1.Leechers[testLeecher1.ID] + if foundLeecher != *testLeecher1 { + t.Error("leecher not updated in local", testLeecher1) + } +} + +func TestLeecherFinished(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent1)) + testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) + tx.AddLeecher(testTorrent1, testLeecher1) + testLeecher1.Left = 0 + + tx.LeecherFinished(testTorrent1, testLeecher1) + foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + foundSeeder, _ := foundTorrent.Seeders[testLeecher1.ID] + if foundSeeder != *testLeecher1 { + t.Error("seeder not added to cache", testLeecher1, foundSeeder) + } + foundSeeder, _ = foundTorrent.Leechers[testLeecher1.ID] + if foundSeeder == *testLeecher1 { + t.Error("leecher not removed from cache", testLeecher1) + } + foundSeeder, _ = testTorrent1.Seeders[testLeecher1.ID] + if foundSeeder != *testLeecher1 { + t.Error("seeder not added to local", testLeecher1) + } + foundSeeder, _ = testTorrent1.Leechers[testLeecher1.ID] + if foundSeeder == *testLeecher1 { + t.Error("leecher not removed from local", testLeecher1) + } +} + +// Add, update, verify remove +func TestUpdatePeer(t *testing.T) { + tx := createTestTx() + testTorrent1 := createTestTorrent() + testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) + panicErrNil(tx.AddTorrent(testTorrent1)) + panicErrNil(tx.AddSeeder(testTorrent1, testSeeder1)) // Update a seeder, set it, then check to make sure it updated r := rand.New(rand.NewSource(time.Now().UnixNano())) - testSeeder1.Downloaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(&testTorrent1, &testSeeder1)) - foundTorrent, found, err = tx.FindTorrent(testTorrent1.Infohash) - panicErrNil(err) - if seeder1, exists := foundTorrent.Seeders[testSeeder1.ID]; !exists { - t.Error("seeder not added") - } else if seeder1 != testSeeder1 { - t.Errorf("seeder changed from %v to %v", testSeeder1, seeder1) - } + testSeeder1.Uploaded += uint64(r.Int63()) + panicErrNil(tx.SetSeeder(testTorrent1, testSeeder1)) + + panicErrNil(tx.RemoveSeeder(testTorrent1, testSeeder1)) + foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(err) + if seeder1, exists := foundTorrent.Seeders[testSeeder1.ID]; exists { + t.Error("seeder not removed from cache", seeder1) + } + if seeder1, exists := testTorrent1.Seeders[testSeeder1.ID]; exists { + t.Error("seeder not removed from local", seeder1) + } } diff --git a/server/announce.go b/server/announce.go index 150ca19..3a37e81 100644 --- a/server/announce.go +++ b/server/announce.go @@ -170,11 +170,7 @@ func (s Server) serveAnnounce(w http.ResponseWriter, r *http.Request) { log.Panicf("server: %s", err) } if leecher { - err := tx.RemoveLeecher(torrent, peer) - if err != nil { - log.Panicf("server: %s", err) - } - err = tx.AddSeeder(torrent, peer) + err := tx.LeecherFinished(torrent, peer) if err != nil { log.Panicf("server: %s", err) } @@ -182,11 +178,7 @@ func (s Server) serveAnnounce(w http.ResponseWriter, r *http.Request) { case leecher && left == 0: // A leecher completed but the event was never received - err := tx.RemoveLeecher(torrent, peer) - if err != nil { - log.Panicf("server: %s", err) - } - err = tx.AddSeeder(torrent, peer) + err := tx.LeecherFinished(torrent, peer) if err != nil { log.Panicf("server: %s", err) } From 1ea24f80dccf07ae114858ff7f43cb33d7c05632 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Mon, 16 Sep 2013 00:06:48 -0400 Subject: [PATCH 4/9] Completed cache.Tx tests and added benchmarks --- cache/redis/redis.go | 67 +++--- cache/redis/redis_bench_test.go | 336 +++++++++++++++++++---------- cache/redis/redis_test.go | 68 +++--- cache/redis/tx_test.go | 367 +++++++++++++++++++------------- models/models.go | 8 + server/announce.go | 4 +- 6 files changed, 527 insertions(+), 323 deletions(-) diff --git a/cache/redis/redis.go b/cache/redis/redis.go index 90e3800..dd3d245 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -89,9 +89,8 @@ func (p *Pool) Get() (cache.Tx, error) { } type Tx struct { - conf *config.DataStore - done bool - multi bool + conf *config.DataStore + done bool redis.Conn } @@ -107,7 +106,6 @@ func createUser(userVals []string) (*models.User, error) { if len(userVals) != 7 { return nil, ErrCreateUser } - // This could be a loop+switch ID, err := strconv.ParseUint(userVals[0], 10, 64) if err != nil { return nil, err @@ -133,7 +131,8 @@ func createUser(userVals []string) (*models.User, error) { if err != nil { return nil, err } - return &models.User{ID, Passkey, UpMultiplier, DownMultiplier, Slots, SlotsUsed, uint(Snatches)}, nil + return &models.User{ID: ID, Passkey: Passkey, UpMultiplier: UpMultiplier, + DownMultiplier: DownMultiplier, Slots: Slots, SlotsUsed: SlotsUsed, Snatches: uint(Snatches)}, nil } // This is a mulple action command, it's not internally atomic @@ -175,7 +174,8 @@ func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { return nil, err } - return &models.Torrent{ID, Infohash, Active, seeders, leechers, uint(Snatches), UpMultiplier, DownMultiplier, LastAction}, nil + return &models.Torrent{ID: ID, Infohash: Infohash, Active: Active, Seeders: seeders, Leechers: leechers, + Snatches: uint(Snatches), UpMultiplier: UpMultiplier, DownMultiplier: DownMultiplier, LastAction: LastAction}, nil } // The peer hashkey relies on the combination of peerID, userID, and torrentID being unique @@ -220,7 +220,7 @@ func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTy } delete(peers, peer.ID) } - // Only delete the set if all the peer deletions were successful + // Will only delete the set if all the peer deletions were successful setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) _, err := tx.Do("DEL", setKey) if err != nil { @@ -234,8 +234,8 @@ func getPeerHashKey(peer *models.Peer) string { return peer.ID + ":" + strconv.FormatUint(peer.UserID, 36) + ":" + strconv.FormatUint(peer.TorrentID, 36) } -func getPeerSetKey(prefix string, peer *models.Peer) string { - return prefix + strconv.FormatUint(peer.TorrentID, 36) +func getPeerSetKey(typePrefix string, peer *models.Peer) string { + return typePrefix + strconv.FormatUint(peer.TorrentID, 36) } // This is a mulple action command, it's not internally atomic @@ -286,7 +286,8 @@ func createPeer(peerVals []string) (*models.Peer, error) { if err != nil { return nil, err } - return &models.Peer{ID, UserID, TorrentID, IP, Port, Uploaded, Downloaded, Left, LastAnnounce}, nil + return &models.Peer{ID: ID, UserID: UserID, TorrentID: TorrentID, IP: IP, Port: Port, + Uploaded: Uploaded, Downloaded: Downloaded, Left: Left, LastAnnounce: LastAnnounce}, nil } @@ -309,7 +310,7 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin if err != nil { return nil, err } - peers[peer.ID] = *peer + peers[models.PeerMapKey(peer)] = *peer } return } @@ -356,7 +357,6 @@ func (tx *Tx) RemoveTorrent(t *models.Torrent) error { if err != nil { return err } - return nil } @@ -438,14 +438,14 @@ func (tx *Tx) UnWhitelistClient(peerID string) error { func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { torrentKey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash - snatchCount, err := redis.Int(tx.Do("HINCRBY", torrentKey, 1)) + snatchCount, err := redis.Int(tx.Do("HINCRBY", torrentKey, "snatches", 1)) if err != nil { return err } torrent.Snatches = uint(snatchCount) - userKey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash - snatchCount, err = redis.Int(tx.Do("HINCRBY", userKey, 1)) + userKey := tx.conf.Prefix + UserPrefix + user.Passkey + snatchCount, err = redis.Int(tx.Do("HINCRBY", userKey, "snatches", 1)) if err != nil { return err } @@ -455,10 +455,25 @@ func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { func (tx *Tx) MarkActive(torrent *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash - activeExists, err := redis.Int(tx.Do("HSET", hashkey, true)) + activeExists, err := redis.Int(tx.Do("HSET", hashkey, "active", true)) if err != nil { return err } + torrent.Active = true + // HSET returns 1 if hash didn't exist before + if activeExists == 1 { + return ErrMarkActive + } + return nil +} + +func (tx *Tx) MarkInActive(torrent *models.Torrent) error { + hashkey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash + activeExists, err := redis.Int(tx.Do("HSET", hashkey, "active", false)) + if err != nil { + return err + } + torrent.Active = false // HSET returns 1 if hash didn't exist before if activeExists == 1 { return ErrMarkActive @@ -480,7 +495,7 @@ func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { if torrent.Leechers == nil { torrent.Leechers = make(map[string]models.Peer) } - torrent.Leechers[peer.ID] = *peer + torrent.Leechers[models.PeerMapKey(peer)] = *peer return nil } @@ -491,7 +506,7 @@ func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { if err != nil { return err } - t.Leechers[p.ID] = *p + t.Leechers[models.PeerMapKey(p)] = *p return nil } @@ -500,7 +515,7 @@ func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { if err != nil { return err } - delete(t.Leechers, p.ID) + delete(t.Leechers, models.PeerMapKey(p)) return nil } @@ -513,8 +528,8 @@ func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error if err != nil { return err } - torrent.Seeders[peer.ID] = *peer - delete(torrent.Leechers, peer.ID) + torrent.Seeders[models.PeerMapKey(peer)] = *peer + delete(torrent.Leechers, models.PeerMapKey(peer)) err = tx.setPeer(peer) return err @@ -534,7 +549,7 @@ func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { if torrent.Seeders == nil { torrent.Seeders = make(map[string]models.Peer) } - torrent.Seeders[peer.ID] = *peer + torrent.Seeders[models.PeerMapKey(peer)] = *peer return nil } @@ -543,7 +558,7 @@ func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { if err != nil { return err } - t.Seeders[p.ID] = *p + t.Seeders[models.PeerMapKey(p)] = *p return nil } @@ -552,13 +567,13 @@ func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { if err != nil { return err } - delete(t.Seeders, p.ID) + delete(t.Seeders, models.PeerMapKey(p)) return nil } func (tx *Tx) IncrementSlots(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey - slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, 1)) + slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, "slots", 1)) if err != nil { return err } @@ -568,7 +583,7 @@ func (tx *Tx) IncrementSlots(u *models.User) error { func (tx *Tx) DecrementSlots(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey - slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, -1)) + slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, "slots", -1)) if err != nil { return err } diff --git a/cache/redis/redis_bench_test.go b/cache/redis/redis_bench_test.go index 7c14bee..61bd0da 100644 --- a/cache/redis/redis_bench_test.go +++ b/cache/redis/redis_bench_test.go @@ -6,128 +6,240 @@ package redis import ( - "errors" + "math/rand" "testing" - - "github.com/garyburd/redigo/redis" - - "github.com/pushrax/chihaya/models" + "time" ) -var ( - ErrTxDone = errors.New("cache: Transaction has already been committed or rolled back") - ErrTxConflict = errors.New("cache: Commit interrupted, update transaction and repeat") -) - -// Maximum number of parallel retries; depends on system latency -const MAX_RETRIES = 9000 - -// Legacy JSON support for benching -func (tx *Tx) initiateWrite() error { - if tx.done { - return ErrTxDone - } - if tx.multi != true { - tx.multi = true - return tx.Send("MULTI") - } - return nil -} - -func (tx *Tx) initiateRead() error { - if tx.done { - return ErrTxDone - } - if tx.multi == true { - panic("Tried to read during MULTI") - } - return nil -} - -func (tx *Tx) Commit() error { - if tx.done { - return ErrTxDone - } - if tx.multi == true { - execResponse, err := tx.Do("EXEC") - if execResponse == nil { - tx.multi = false - return ErrTxConflict - } - if err != nil { - return err - } - } - tx.close() - return nil -} - -func (tx *Tx) Rollback() error { - if tx.done { - return ErrTxDone - } - // Undoes watches and multi - if _, err := tx.Do("DISCARD"); err != nil { - return err - } - tx.multi = false - tx.close() - return nil -} - -func ExampleRedisTypesSchemaFindUser(passkey string, t TestReporter) (*models.User, bool) { - testTx := createTestTxObj(t) - hashkey := testTx.conf.Prefix + UserPrefix + passkey - userVals, err := redis.Strings(testTx.Do("HVALS", hashkey)) - if len(userVals) == 0 { - return nil, false - } - verifyErrNil(err, t) - compareUser, err := createUser(userVals) - verifyErrNil(err, t) - return compareUser, true -} - -func BenchmarkRedisTypesSchemaRemoveSeeder(b *testing.B) { - for bCount := 0; bCount < b.N; bCount++ { - //TODO this needs to be updated - b.Error("Unimplemented") - - } -} - -func BenchmarkRedisTypesSchemaFindUser(b *testing.B) { - - // Ensure successful user find ( a failed lookup may have different performance ) +func BenchmarkSuccessfulFindUser(b *testing.B) { b.StopTimer() + tx := createTestTx() testUser := createTestUser() - testTx := createTestTxObj(b) - hashkey := testTx.conf.Prefix + UserPrefix + testUser.Passkey - reply, err := testTx.Do("HMSET", hashkey, - "id", testUser.ID, - "passkey", testUser.Passkey, - "up_multiplier", testUser.UpMultiplier, - "down_multiplier", testUser.DownMultiplier, - "slots", testUser.Slots, - "slots_used", testUser.SlotsUsed) - - if reply == nil { - b.Log("no hash fields added!") - } - verifyErrNil(err, b) + panicErrNil(tx.AddUser(testUser)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { - compareUser, exists := ExampleRedisTypesSchemaFindUser(testUser.Passkey, b) - - b.StopTimer() - if !exists { - b.Error("User not found!") + foundUser, found, err := tx.FindUser(testUser.Passkey) + panicErrNil(err) + if !found { + b.Error("user not found", testUser) } - if testUser != *compareUser { - b.Errorf("user mismatch: %v vs. %v", compareUser, testUser) + if *foundUser != *testUser { + b.Error("found user mismatch", *foundUser, testUser) } - b.StartTimer() } } + +func BenchmarkFailedFindUser(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testUser := createTestUser() + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + + _, found, err := tx.FindUser(testUser.Passkey) + panicErrNil(err) + if found { + b.Error("user not found", testUser) + } + } +} + +func BenchmarkSuccessfulFindTorrent(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + + panicErrNil(tx.AddTorrent(testTorrent)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) + panicErrNil(err) + if !found { + b.Error("torrent not found", testTorrent) + } + // Incomplete comparison as maps make struct not nativly comparable + if foundTorrent.Infohash != testTorrent.Infohash { + b.Error("found torrent mismatch", foundTorrent, testTorrent) + } + } +} + +func BenchmarkFailFindTorrent(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) + panicErrNil(err) + if found { + b.Error("torrent found", foundTorrent) + } + } +} + +func BenchmarkSuccessfulClientWhitelisted(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testPeerID := "-lt0D30-" + panicErrNil(tx.WhitelistClient(testPeerID)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + found, err := tx.ClientWhitelisted(testPeerID) + panicErrNil(err) + if !found { + b.Error("peerID not found", testPeerID) + } + } +} + +func BenchmarkFailClientWhitelisted(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testPeerID2 := "TIX0192" + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + found, err := tx.ClientWhitelisted(testPeerID2) + panicErrNil(err) + if found { + b.Error("peerID found", testPeerID2) + } + } +} + +func BenchmarkRecordSnatch(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + testUser := createTestUser() + panicErrNil(tx.AddTorrent(testTorrent)) + panicErrNil(tx.AddUser(testUser)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + panicErrNil(tx.RecordSnatch(testUser, testTorrent)) + } +} + +func BenchmarkMarkActive(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + testTorrent.Active = false + panicErrNil(tx.AddTorrent(testTorrent)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + panicErrNil(tx.MarkActive(testTorrent)) + } +} + +func BenchmarkAddSeeder(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + b.StopTimer() + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) + b.StartTimer() + + panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + } +} + +func BenchmarkRemoveSeeder(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + b.StopTimer() + tx.AddSeeder(testTorrent, testSeeder) + b.StartTimer() + + panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + } +} + +func BenchmarkSetSeeder(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + r := rand.New(rand.NewSource(time.Now().UnixNano())) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + b.StopTimer() + testSeeder.Uploaded += uint64(r.Int63()) + b.StartTimer() + + tx.SetSeeder(testTorrent, testSeeder) + } +} + +func BenchmarkIncrementSlots(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testUser := createTestUser() + panicErrNil(tx.AddUser(testUser)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + panicErrNil(tx.IncrementSlots(testUser)) + } +} + +func BenchmarkLeecherFinished(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + b.StopTimer() + testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + testLeecher.Left = 0 + b.StartTimer() + + panicErrNil(tx.LeecherFinished(testTorrent, testLeecher)) + } +} + +// This is a comparision to the Leecher finished function +func BenchmarkRemoveLeecherAddSeeder(b *testing.B) { + b.StopTimer() + tx := createTestTx() + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + b.StartTimer() + + for bCount := 0; bCount < b.N; bCount++ { + b.StopTimer() + testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + testLeecher.Left = 0 + b.StartTimer() + + panicErrNil(tx.RemoveLeecher(testTorrent, testLeecher)) + panicErrNil(tx.AddSeeder(testTorrent, testLeecher)) + } + +} diff --git a/cache/redis/redis_test.go b/cache/redis/redis_test.go index 777cfa8..b78aac1 100644 --- a/cache/redis/redis_test.go +++ b/cache/redis/redis_test.go @@ -6,6 +6,7 @@ package redis import ( "crypto/rand" + "fmt" "io" "os" "strconv" @@ -64,16 +65,17 @@ type TestReporter interface { Logf(format string, args ...interface{}) } -func verifyErrNil(err error, t TestReporter) { +func panicErrNil(err error) { if err != nil { - t.Error(err) + fmt.Println(err) + panic(err) } } -func createTestTxObj(t TestReporter) *Tx { +func createTestTxObj() *Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) conf := &testConfig.Cache - verifyErrNil(err, t) + panicErrNil(err) testPool := &Pool{ conf: conf, @@ -86,27 +88,27 @@ func createTestTxObj(t TestReporter) *Tx { } txObj := &Tx{ - conf: testPool.conf, - done: false, - multi: false, - Conn: testPool.pool.Get(), + conf: testPool.conf, + done: false, + Conn: testPool.pool.Get(), } - verifyErrNil(err, t) + panicErrNil(err) // Test connection before returning _, err = txObj.Do("PING") - verifyErrNil(err, t) + panicErrNil(err) return txObj } -func createTestUser() models.User { - testUser := models.User{createTestUserID(), createTestPasskey(), 1.01, 1.0, 4, 2, 7} - return testUser +func createTestUser() *models.User { + return &models.User{ID: createTestUserID(), Passkey: createTestPasskey(), + UpMultiplier: 1.01, DownMultiplier: 1.0, Slots: 4, SlotsUsed: 2, Snatches: 7} } func createTestPeer(userID uint64, torrentID uint64) *models.Peer { - return &models.Peer{createTestPeerID(), userID, torrentID, "127.0.0.1", 6889, 1024, 3000, 4200, 11} + return &models.Peer{ID: createTestPeerID(), UserID: userID, TorrentID: torrentID, + IP: "127.0.0.1", Port: 6889, Uploaded: 1024, Downloaded: 3000, Left: 4200, LastAnnounce: 11} } func createTestPeers(torrentID uint64, num int) map[string]models.Peer { @@ -126,36 +128,22 @@ func createTestTorrent() *models.Torrent { testSeeders := createTestPeers(torrentID, 4) testLeechers := createTestPeers(torrentID, 2) - testTorrent := models.Torrent{torrentID, torrentInfohash, true, testSeeders, testLeechers, 11, 0.0, 0.0, 0} + testTorrent := models.Torrent{ID: torrentID, Infohash: torrentInfohash, Active: true, + Seeders: testSeeders, Leechers: testLeechers, Snatches: 11, UpMultiplier: 1.0, DownMultiplier: 1.0, LastAction: 0} return &testTorrent } -func TestAddGetPeers(t *testing.T) { +func TestPeersAlone(t *testing.T) { - testTx := createTestTxObj(t) - testTorrent := createTestTorrent() + testTx := createTestTxObj() + testTorrentID := createTestTorrentID() + testPeers := createTestPeers(testTorrentID, 3) - setkey := testTx.conf.Prefix + SeederPrefix + strconv.FormatUint(testTorrent.ID, 36) - testTx.Do("DEL", setkey) - - testTx.addPeers(testTorrent.Seeders, SeederPrefix) - peerMap, err := testTx.getPeers(testTorrent.ID, SeederPrefix) - if err != nil { - t.Error(err) - } else if len(peerMap) != len(testTorrent.Seeders) { - t.Error("Num Peers not equal") + panicErrNil(testTx.addPeers(testPeers, "test:")) + peerMap, err := testTx.getPeers(testTorrentID, "test:") + panicErrNil(err) + if len(peerMap) != len(testPeers) { + t.Error("Num Peers not equal ", len(peerMap), len(testPeers)) } -} - -func TestCloseClosedTransaction(t *testing.T) { - //require panic - defer func() { - if err := recover(); err == nil { - t.Error("Closing a closed transaction did not panic") - } - }() - - testTx := createTestTxObj(t) - testTx.close() - testTx.close() + panicErrNil(testTx.removePeers(testTorrentID, testPeers, "test:")) } diff --git a/cache/redis/tx_test.go b/cache/redis/tx_test.go index f5d50c9..76ff5ac 100644 --- a/cache/redis/tx_test.go +++ b/cache/redis/tx_test.go @@ -5,7 +5,6 @@ package redis import ( - "fmt" "math/rand" "os" "testing" @@ -13,15 +12,9 @@ import ( "github.com/pushrax/chihaya/cache" "github.com/pushrax/chihaya/config" + "github.com/pushrax/chihaya/models" ) -func panicErrNil(err error) { - if err != nil { - fmt.Println(err) - panic(err) - } -} - func createTestTx() cache.Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) panicErrNil(err) @@ -38,24 +31,24 @@ func createTestTx() cache.Tx { func TestFindUserSuccess(t *testing.T) { tx := createTestTx() - testUser1 := createTestUser() + testUser := createTestUser() - panicErrNil(tx.AddUser(&testUser1)) - foundUser, found, err := tx.FindUser(testUser1.Passkey) + panicErrNil(tx.AddUser(testUser)) + foundUser, found, err := tx.FindUser(testUser.Passkey) panicErrNil(err) if !found { - t.Error("user not found", testUser1) + t.Error("user not found", testUser) } - if *foundUser != testUser1 { - t.Error("found user mismatch", *foundUser, testUser1) + if *foundUser != *testUser { + t.Error("found user mismatch", *foundUser, testUser) } } func TestFindUserFail(t *testing.T) { tx := createTestTx() - testUser2 := createTestUser() + testUser := createTestUser() - foundUser, found, err := tx.FindUser(testUser2.Passkey) + foundUser, found, err := tx.FindUser(testUser.Passkey) panicErrNil(err) if found { t.Error("user found", foundUser) @@ -64,12 +57,12 @@ func TestFindUserFail(t *testing.T) { func TestRemoveUser(t *testing.T) { tx := createTestTx() - testUser1 := createTestUser() + testUser := createTestUser() - panicErrNil(tx.AddUser(&testUser1)) - err := tx.RemoveUser(&testUser1) + panicErrNil(tx.AddUser(testUser)) + err := tx.RemoveUser(testUser) panicErrNil(err) - foundUser, found, err := tx.FindUser(testUser1.Passkey) + foundUser, found, err := tx.FindUser(testUser.Passkey) panicErrNil(err) if found { t.Error("removed user found", foundUser) @@ -78,25 +71,25 @@ func TestRemoveUser(t *testing.T) { func TestFindTorrent(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() + testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.AddTorrent(testTorrent)) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) if !found { - t.Error("torrent not found", testTorrent1) + t.Error("torrent not found", testTorrent) } // Incomplete comparison as maps make struct not nativly comparable - if foundTorrent.Infohash != testTorrent1.Infohash { - t.Error("found torrent mismatch", foundTorrent, testTorrent1) + if foundTorrent.Infohash != testTorrent.Infohash { + t.Error("found torrent mismatch", foundTorrent, testTorrent) } } func TestFindTorrentFail(t *testing.T) { tx := createTestTx() - testTorrent2 := createTestTorrent() + testTorrent := createTestTorrent() - foundTorrent, found, err := tx.FindTorrent(testTorrent2.Infohash) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) if found { t.Error("torrent found", foundTorrent) @@ -105,11 +98,11 @@ func TestFindTorrentFail(t *testing.T) { func TestRemoveTorrent(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) - panicErrNil(tx.RemoveTorrent(testTorrent1)) - foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.RemoveTorrent(testTorrent)) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) if found { t.Error("removed torrent found", foundTorrent) @@ -118,13 +111,13 @@ func TestRemoveTorrent(t *testing.T) { func TestClientWhitelistSuccess(t *testing.T) { tx := createTestTx() - testPeerID1 := "-lt0D30-" + testPeerID := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID1)) - found, err := tx.ClientWhitelisted(testPeerID1) + panicErrNil(tx.WhitelistClient(testPeerID)) + found, err := tx.ClientWhitelisted(testPeerID) panicErrNil(err) if !found { - t.Error("peerID not found", testPeerID1) + t.Error("peerID not found", testPeerID) } } @@ -137,194 +130,282 @@ func TestClientWhitelistFail(t *testing.T) { if found { t.Error("peerID found", testPeerID2) } +} +func TestRecordSnatch(t *testing.T) { + tx := createTestTx() + testTorrent := createTestTorrent() + testUser := createTestUser() + panicErrNil(tx.AddTorrent(testTorrent)) + panicErrNil(tx.AddUser(testUser)) + + userSnatches := testUser.Snatches + torrentSnatches := testTorrent.Snatches + + panicErrNil(tx.RecordSnatch(testUser, testTorrent)) + + foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) + panicErrNil(err) + foundUser, _, err := tx.FindUser(testUser.Passkey) + panicErrNil(err) + + if testUser.Snatches != userSnatches+1 { + t.Error("snatch not recorded to local user", testUser.Snatches, userSnatches+1) + } + if testTorrent.Snatches != torrentSnatches+1 { + t.Error("snatch not recorded to local torrent") + } + if foundUser.Snatches != userSnatches+1 { + t.Error("snatch not recorded to cached user", foundUser.Snatches, userSnatches+1) + } + if foundTorrent.Snatches != torrentSnatches+1 { + t.Error("snatch not recorded to cached torrent") + } +} + +func TestMarkActive(t *testing.T) { + tx := createTestTx() + testTorrent := createTestTorrent() + testTorrent.Active = false + panicErrNil(tx.AddTorrent(testTorrent)) + + panicErrNil(tx.MarkActive(testTorrent)) + foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) + panicErrNil(err) + + if foundTorrent.Active != true { + t.Error("cached torrent not activated") + } + if testTorrent.Active != true { + t.Error("cached torrent not activated") + } } func TestClientWhitelistRemove(t *testing.T) { tx := createTestTx() - testPeerID1 := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID1)) - panicErrNil(tx.UnWhitelistClient(testPeerID1)) + testPeerID := "-lt0D30-" + panicErrNil(tx.WhitelistClient(testPeerID)) + panicErrNil(tx.UnWhitelistClient(testPeerID)) - found, err := tx.ClientWhitelisted(testPeerID1) + found, err := tx.ClientWhitelisted(testPeerID) panicErrNil(err) if found { - t.Error("removed peerID found", testPeerID1) + t.Error("removed peerID found", testPeerID) } } func TestAddSeeder(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent1, testSeeder1)) - foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - foundSeeder, found := foundTorrent.Seeders[testSeeder1.ID] - if found && foundSeeder != *testSeeder1 { - t.Error("seeder not added to cache", testSeeder1) + foundSeeder, found := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] + if found && foundSeeder != *testSeeder { + t.Error("seeder not added to cache", testSeeder) } - foundSeeder, found = testTorrent1.Seeders[testSeeder1.ID] - if found && foundSeeder != *testSeeder1 { - t.Error("seeder not added to local", testSeeder1) + foundSeeder, found = testTorrent.Seeders[models.PeerMapKey(testSeeder)] + if found && foundSeeder != *testSeeder { + t.Error("seeder not added to local", testSeeder) } } func TestAddLeecher(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - tx.AddLeecher(testTorrent1, testLeecher1) - foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - foundLeecher, found := foundTorrent.Leechers[testLeecher1.ID] - if found && foundLeecher != *testLeecher1 { - t.Error("leecher not added to cache", testLeecher1) + foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] + if found && foundLeecher != *testLeecher { + t.Error("leecher not added to cache", testLeecher) } - foundLeecher, found = testTorrent1.Leechers[testLeecher1.ID] - if found && foundLeecher != *testLeecher1 { - t.Error("leecher not added to local", testLeecher1) + foundLeecher, found = testTorrent.Leechers[models.PeerMapKey(testLeecher)] + if found && foundLeecher != *testLeecher { + t.Error("leecher not added to local", testLeecher) } } func TestRemoveSeeder(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) - tx.AddSeeder(testTorrent1, testSeeder1) + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) - panicErrNil(tx.RemoveSeeder(testTorrent1, testSeeder1)) - foundSeeder, found := testTorrent1.Seeders[testSeeder1.ID] - if found || foundSeeder == *testSeeder1 { + panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + foundSeeder, found := testTorrent.Seeders[models.PeerMapKey(testSeeder)] + if found || foundSeeder == *testSeeder { t.Error("seeder not removed from local", foundSeeder) } - foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - foundSeeder, found = foundTorrent.Seeders[testSeeder1.ID] - if found || foundSeeder == *testSeeder1 { - t.Error("seeder not removed from cache", foundSeeder) + foundSeeder, found = foundTorrent.Seeders[models.PeerMapKey(testSeeder)] + if found || foundSeeder == *testSeeder { + t.Error("seeder not removed from cache", foundSeeder, *testSeeder) } } func TestRemoveLeecher(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) - tx.AddLeecher(testTorrent1, testLeecher1) + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) - tx.RemoveLeecher(testTorrent1, testLeecher1) - foundTorrent, found, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.RemoveLeecher(testTorrent, testLeecher)) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - foundLeecher, found := foundTorrent.Leechers[testLeecher1.ID] - if found || foundLeecher == *testLeecher1 { - t.Error("leecher not removed from cache", foundLeecher) + foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] + if found || foundLeecher == *testLeecher { + t.Error("leecher not removed from cache", foundLeecher, *testLeecher) } - foundLeecher, found = testTorrent1.Leechers[testLeecher1.ID] - if found || foundLeecher == *testLeecher1 { - t.Error("leecher not removed from local", foundLeecher) + foundLeecher, found = testTorrent.Leechers[models.PeerMapKey(testLeecher)] + if found || foundLeecher == *testLeecher { + t.Error("leecher not removed from local", foundLeecher, *testLeecher) } } func TestSetSeeder(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) - tx.AddSeeder(testTorrent1, testSeeder1) + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) - testSeeder1.Uploaded += 100 + r := rand.New(rand.NewSource(time.Now().UnixNano())) + testSeeder.Uploaded += uint64(r.Int63()) - tx.SetSeeder(testTorrent1, testSeeder1) - foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + + foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - foundSeeder, _ := foundTorrent.Seeders[testSeeder1.ID] - if foundSeeder != *testSeeder1 { - t.Error("seeder not updated in cache", testSeeder1) + foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] + if foundSeeder != *testSeeder { + t.Error("seeder not updated in cache", foundSeeder, *testSeeder) } - foundSeeder, _ = testTorrent1.Seeders[testSeeder1.ID] - if foundSeeder != *testSeeder1 { - t.Error("seeder not updated in local", testSeeder1) + foundSeeder, _ = testTorrent.Seeders[models.PeerMapKey(testSeeder)] + if foundSeeder != *testSeeder { + t.Error("seeder not updated in local", foundSeeder, *testSeeder) } } func TestSetLeecher(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) - tx.AddLeecher(testTorrent1, testLeecher1) + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) - testLeecher1.Uploaded += 100 + r := rand.New(rand.NewSource(time.Now().UnixNano())) + testLeecher.Uploaded += uint64(r.Int63()) - tx.SetLeecher(testTorrent1, testLeecher1) - foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.SetLeecher(testTorrent, testLeecher)) + foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - foundLeecher, _ := foundTorrent.Leechers[testLeecher1.ID] - if foundLeecher != *testLeecher1 { - t.Error("leecher not updated in cache", testLeecher1) + foundLeecher, _ := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] + if foundLeecher != *testLeecher { + t.Error("leecher not updated in cache", testLeecher) } - foundLeecher, _ = testTorrent1.Leechers[testLeecher1.ID] - if foundLeecher != *testLeecher1 { - t.Error("leecher not updated in local", testLeecher1) + foundLeecher, _ = testTorrent.Leechers[models.PeerMapKey(testLeecher)] + if foundLeecher != *testLeecher { + t.Error("leecher not updated in local", testLeecher) + } +} + +func TestIncrementSlots(t *testing.T) { + tx := createTestTx() + testUser := createTestUser() + panicErrNil(tx.AddUser(testUser)) + numSlots := testUser.Slots + + panicErrNil(tx.IncrementSlots(testUser)) + foundUser, _, err := tx.FindUser(testUser.Passkey) + panicErrNil(err) + + if foundUser.Slots != numSlots+1 { + t.Error("cached slots not incremented") + } + if testUser.Slots != numSlots+1 { + t.Error("local slots not incremented") + } +} + +func TestDecrementSlots(t *testing.T) { + tx := createTestTx() + testUser := createTestUser() + panicErrNil(tx.AddUser(testUser)) + numSlots := testUser.Slots + + panicErrNil(tx.DecrementSlots(testUser)) + foundUser, _, err := tx.FindUser(testUser.Passkey) + panicErrNil(err) + + if foundUser.Slots != numSlots-1 { + t.Error("cached slots not incremented") + } + if testUser.Slots != numSlots-1 { + t.Error("local slots not incremented") } } func TestLeecherFinished(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent1)) - testLeecher1 := createTestPeer(createTestUserID(), testTorrent1.ID) - tx.AddLeecher(testTorrent1, testLeecher1) - testLeecher1.Left = 0 + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + testLeecher.Left = 0 - tx.LeecherFinished(testTorrent1, testLeecher1) - foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.LeecherFinished(testTorrent, testLeecher)) + + foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - foundSeeder, _ := foundTorrent.Seeders[testLeecher1.ID] - if foundSeeder != *testLeecher1 { - t.Error("seeder not added to cache", testLeecher1, foundSeeder) + foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testLeecher)] + if foundSeeder != *testLeecher { + t.Error("seeder not added to cache", foundSeeder, *testLeecher) } - foundSeeder, _ = foundTorrent.Leechers[testLeecher1.ID] - if foundSeeder == *testLeecher1 { - t.Error("leecher not removed from cache", testLeecher1) + foundSeeder, _ = foundTorrent.Leechers[models.PeerMapKey(testLeecher)] + if foundSeeder == *testLeecher { + t.Error("leecher not removed from cache", testLeecher) } - foundSeeder, _ = testTorrent1.Seeders[testLeecher1.ID] - if foundSeeder != *testLeecher1 { - t.Error("seeder not added to local", testLeecher1) + foundSeeder, _ = testTorrent.Seeders[models.PeerMapKey(testLeecher)] + if foundSeeder != *testLeecher { + t.Error("seeder not added to local", testLeecher) } - foundSeeder, _ = testTorrent1.Leechers[testLeecher1.ID] - if foundSeeder == *testLeecher1 { - t.Error("leecher not removed from local", testLeecher1) + foundSeeder, _ = testTorrent.Leechers[models.PeerMapKey(testLeecher)] + if foundSeeder == *testLeecher { + t.Error("leecher not removed from local", testLeecher) } } // Add, update, verify remove func TestUpdatePeer(t *testing.T) { tx := createTestTx() - testTorrent1 := createTestTorrent() - testSeeder1 := createTestPeer(createTestUserID(), testTorrent1.ID) - panicErrNil(tx.AddTorrent(testTorrent1)) - panicErrNil(tx.AddSeeder(testTorrent1, testSeeder1)) + testTorrent := createTestTorrent() + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddTorrent(testTorrent)) + panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) // Update a seeder, set it, then check to make sure it updated r := rand.New(rand.NewSource(time.Now().UnixNano())) - testSeeder1.Uploaded += uint64(r.Int63()) + testSeeder.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(testTorrent1, testSeeder1)) + panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) - panicErrNil(tx.RemoveSeeder(testTorrent1, testSeeder1)) - foundTorrent, _, err := tx.FindTorrent(testTorrent1.Infohash) + panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) - if seeder1, exists := foundTorrent.Seeders[testSeeder1.ID]; exists { - t.Error("seeder not removed from cache", seeder1) + if seeder, exists := foundTorrent.Seeders[models.PeerMapKey(testSeeder)]; exists { + t.Error("seeder not removed from cache", seeder) } - if seeder1, exists := testTorrent1.Seeders[testSeeder1.ID]; exists { - t.Error("seeder not removed from local", seeder1) + if seeder, exists := testTorrent.Seeders[models.PeerMapKey(testSeeder)]; exists { + t.Error("seeder not removed from local", seeder) } } diff --git a/models/models.go b/models/models.go index cc1def7..1162321 100644 --- a/models/models.go +++ b/models/models.go @@ -4,6 +4,10 @@ package models +import ( + "strconv" +) + type Peer struct { ID string `json:"id"` UserID uint64 `json:"user_id"` @@ -18,6 +22,10 @@ type Peer struct { LastAnnounce int64 `json:"last_announce"` } +func PeerMapKey(peer *Peer) string { + return peer.ID + ":" + strconv.FormatUint(peer.UserID, 36) +} + type Torrent struct { ID uint64 `json:"id"` Infohash string `json:"infohash"` diff --git a/server/announce.go b/server/announce.go index 3a37e81..90f8d89 100644 --- a/server/announce.go +++ b/server/announce.go @@ -82,8 +82,8 @@ func (s Server) serveAnnounce(w http.ResponseWriter, r *http.Request) { } // Look for the user in in the pool of seeders and leechers - _, seeder := torrent.Seeders[peerID] - _, leecher := torrent.Leechers[peerID] + _, seeder := torrent.Seeders[models.PeerMapKey(peer)] + _, leecher := torrent.Leechers[models.PeerMapKey(peer)] switch { // Guarantee that no user is in both pools From 19900991a780c53a4ad59bd27281d16ede616ce9 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Thu, 19 Sep 2013 00:27:10 -0400 Subject: [PATCH 5/9] Added parallel tests --- cache/redis/redis.go | 7 ++- cache/redis/redis_test.go | 109 ++++++++++++++++++++++++++++------- cache/redis/tx_test.go | 118 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 207 insertions(+), 27 deletions(-) diff --git a/cache/redis/redis.go b/cache/redis/redis.go index dd3d245..b178363 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -297,14 +297,17 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) peerStrings, err := redis.Strings(tx.Do("SMEMBERS", setKey)) if err != nil { - return peers, err + return nil, err } // Keys map to peer objects stored in hashes for _, peerHashKey := range peerStrings { hashKey := tx.conf.Prefix + peerHashKey peerVals, err := redis.Strings(tx.Do("HVALS", hashKey)) if err != nil { - return peers, err + return nil, err + } + if len(peerVals) == 0 { + continue } peer, err := createPeer(peerVals) if err != nil { diff --git a/cache/redis/redis_test.go b/cache/redis/redis_test.go index b78aac1..d670bb0 100644 --- a/cache/redis/redis_test.go +++ b/cache/redis/redis_test.go @@ -19,24 +19,43 @@ import ( ) var ( - testTorrentIDCounter uint64 - testUserIDCounter uint64 - testPeerIDCounter int + testTorrentIDChannel chan uint64 + testUserIDChannel chan uint64 + testPeerIDChannel chan int ) +func init() { + testTorrentIDChannel = make(chan uint64, 100) + testUserIDChannel = make(chan uint64, 100) + testPeerIDChannel = make(chan int, 100) + // Sync access to ID counter with buffered global channels + go func() { + for i := 0; ; i++ { + testTorrentIDChannel <- uint64(i) + } + }() + go func() { + for i := 0; ; i++ { + testUserIDChannel <- uint64(i) + } + }() + go func() { + for i := 0; ; i++ { + testPeerIDChannel <- i + } + }() +} + func createTestTorrentID() uint64 { - testTorrentIDCounter++ - return testTorrentIDCounter + return <-testTorrentIDChannel } func createTestUserID() uint64 { - testUserIDCounter++ - return testUserIDCounter + return <-testUserIDChannel } func createTestPeerID() string { - testPeerIDCounter++ - return "-testPeerID-" + strconv.Itoa(testPeerIDCounter) + return "-testPeerID-" + strconv.Itoa(<-testPeerIDChannel) } func createTestInfohash() string { @@ -57,14 +76,6 @@ func createTestPasskey() string { return string(uuid) } -// Common interface for benchmarks and test error reporting -type TestReporter interface { - Error(args ...interface{}) - Errorf(format string, args ...interface{}) - Log(args ...interface{}) - Logf(format string, args ...interface{}) -} - func panicErrNil(err error) { if err != nil { fmt.Println(err) @@ -72,7 +83,7 @@ func panicErrNil(err error) { } } -func createTestTxObj() *Tx { +func createTestRedisTx() *Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) conf := &testConfig.Cache panicErrNil(err) @@ -115,7 +126,7 @@ func createTestPeers(torrentID uint64, num int) map[string]models.Peer { testPeers := make(map[string]models.Peer) for i := 0; i < num; i++ { tempPeer := createTestPeer(createTestUserID(), torrentID) - testPeers[tempPeer.ID] = *tempPeer + testPeers[models.PeerMapKey(tempPeer)] = *tempPeer } return testPeers } @@ -133,9 +144,43 @@ func createTestTorrent() *models.Torrent { return &testTorrent } -func TestPeersAlone(t *testing.T) { +func comparePeers(lhPeers map[string]models.Peer, rhPeers map[string]models.Peer) bool { + if len(lhPeers) != len(rhPeers) { + return false + } + for rhKey, rhValue := range rhPeers { + lhValue, lhExists := lhPeers[rhKey] + if !lhExists || lhValue != rhValue { + return false + } + } + for lhKey, lhValue := range lhPeers { + rhValue, rhExists := rhPeers[lhKey] + if !rhExists || rhValue != lhValue { + return false + } + } + return true +} - testTx := createTestTxObj() +func torrentsEqual(lhTorrent *models.Torrent, rhTorrent *models.Torrent) bool { + fieldsEqual := lhTorrent.Infohash == rhTorrent.Infohash && + lhTorrent.ID == rhTorrent.ID && + lhTorrent.Active == rhTorrent.Active && + lhTorrent.Snatches == rhTorrent.Snatches && + lhTorrent.UpMultiplier == rhTorrent.UpMultiplier && + lhTorrent.DownMultiplier == rhTorrent.DownMultiplier && + lhTorrent.LastAction == rhTorrent.LastAction + + if !fieldsEqual { + return false + } + + return comparePeers(lhTorrent.Seeders, rhTorrent.Seeders) && comparePeers(lhTorrent.Leechers, rhTorrent.Leechers) +} + +func TestValidPeers(t *testing.T) { + testTx := createTestRedisTx() testTorrentID := createTestTorrentID() testPeers := createTestPeers(testTorrentID, 3) @@ -147,3 +192,25 @@ func TestPeersAlone(t *testing.T) { } panicErrNil(testTx.removePeers(testTorrentID, testPeers, "test:")) } + +func TestInvalidPeers(t *testing.T) { + testTx := createTestRedisTx() + testTorrentID := createTestTorrentID() + testPeers := createTestPeers(testTorrentID, 3) + tempPeer := createTestPeer(createTestUserID(), testTorrentID) + testPeers[models.PeerMapKey(tempPeer)] = *tempPeer + + panicErrNil(testTx.addPeers(testPeers, "test:")) + // Imitate a peer being removed during get + hashKey := testTx.conf.Prefix + getPeerHashKey(tempPeer) + _, err := testTx.Do("DEL", hashKey) + panicErrNil(err) + + peerMap, err := testTx.getPeers(testTorrentID, "test:") + panicErrNil(err) + // Expect 1 less peer due to delete + if len(peerMap) != len(testPeers)-1 { + t.Error("Num Peers not equal ", len(peerMap), len(testPeers)) + } + panicErrNil(testTx.removePeers(testTorrentID, testPeers, "test:")) +} diff --git a/cache/redis/tx_test.go b/cache/redis/tx_test.go index 76ff5ac..f107f1e 100644 --- a/cache/redis/tx_test.go +++ b/cache/redis/tx_test.go @@ -69,18 +69,17 @@ func TestRemoveUser(t *testing.T) { } } -func TestFindTorrent(t *testing.T) { +func TestFindTorrentSuccess(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) panicErrNil(err) if !found { t.Error("torrent not found", testTorrent) } - // Incomplete comparison as maps make struct not nativly comparable - if foundTorrent.Infohash != testTorrent.Infohash { + if !torrentsEqual(foundTorrent, testTorrent) { t.Error("found torrent mismatch", foundTorrent, testTorrent) } } @@ -409,3 +408,114 @@ func TestUpdatePeer(t *testing.T) { t.Error("seeder not removed from local", seeder) } } + +func TestParallelFindUser(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip() + } + tx := createTestTx() + testUserSuccess := createTestUser() + testUserFail := createTestUser() + panicErrNil(tx.AddUser(testUserSuccess)) + + for i := 0; i < 10; i++ { + foundUser, found, err := tx.FindUser(testUserFail.Passkey) + panicErrNil(err) + if found { + t.Error("user found", foundUser) + } + foundUser, found, err = tx.FindUser(testUserSuccess.Passkey) + panicErrNil(err) + if !found { + t.Error("user not found", testUserSuccess) + } + if *foundUser != *testUserSuccess { + t.Error("found user mismatch", *foundUser, testUserSuccess) + } + } +} + +func TestParallelFindTorrent(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip() + } + tx := createTestTx() + testTorrentSuccess := createTestTorrent() + testTorrentFail := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrentSuccess)) + + for i := 0; i < 10; i++ { + foundTorrent, found, err := tx.FindTorrent(testTorrentSuccess.Infohash) + panicErrNil(err) + if !found { + t.Error("torrent not found", testTorrentSuccess) + } + if !torrentsEqual(foundTorrent, testTorrentSuccess) { + t.Error("found torrent mismatch", foundTorrent, testTorrentSuccess) + } + foundTorrent, found, err = tx.FindTorrent(testTorrentFail.Infohash) + panicErrNil(err) + if found { + t.Error("torrent found", foundTorrent) + } + } +} + +func TestParallelSetSeeder(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip() + } + tx := createTestTx() + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) + panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + r := rand.New(rand.NewSource(time.Now().UnixNano())) + + for i := 0; i < 10; i++ { + testSeeder.Uploaded += uint64(r.Int63()) + + panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + + foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) + panicErrNil(err) + foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] + if foundSeeder != *testSeeder { + t.Error("seeder not updated in cache", foundSeeder, *testSeeder) + } + foundSeeder, _ = testTorrent.Seeders[models.PeerMapKey(testSeeder)] + if foundSeeder != *testSeeder { + t.Error("seeder not updated in local", foundSeeder, *testSeeder) + } + } +} + +func TestParallelAddLeecher(t *testing.T) { + t.Parallel() + if testing.Short() { + t.Skip() + } + tx := createTestTx() + testTorrent := createTestTorrent() + panicErrNil(tx.AddTorrent(testTorrent)) + + for i := 0; i < 10; i++ { + testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) + + panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + + foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) + panicErrNil(err) + foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] + if found && foundLeecher != *testLeecher { + t.Error("leecher not added to cache", testLeecher) + } + foundLeecher, found = testTorrent.Leechers[models.PeerMapKey(testLeecher)] + if found && foundLeecher != *testLeecher { + t.Error("leecher not added to local", testLeecher) + } + } +} From 2dd810d2ee6134b63bc6722054a509f57a457fc5 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Tue, 24 Sep 2013 23:10:48 -0400 Subject: [PATCH 6/9] Fixes & format changes from pull request comments --- cache/redis/redis.go | 161 +++++++++++++------------------ cache/redis/redis_bench_test.go | 58 ++++++------ cache/redis/redis_test.go | 62 +++--------- cache/redis/tx_test.go | 163 ++++++++++++++++---------------- models/models.go | 4 +- 5 files changed, 195 insertions(+), 253 deletions(-) diff --git a/cache/redis/redis.go b/cache/redis/redis.go index b178363..af62bd7 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -31,11 +31,11 @@ var ( ErrCreatePeer = errors.New("redis: Incorrect reply length for peer") ErrMarkActive = errors.New("redis: Torrent doesn't exist") - SeederPrefix = "seeders:" - LeecherPrefix = "leechers:" - TorrentPrefix = "torrent:" - UserPrefix = "user:" - PeerPrefix = "peer:" + SeedersPrefix = "seeders:" + LeechersPrefix = "leechers:" + TorrentPrefix = "torrent:" + UserPrefix = "user:" + PeerPrefix = "peer:" ) type driver struct{} @@ -82,10 +82,7 @@ func (p *Pool) Get() (cache.Tx, error) { done: false, Conn: p.pool.Get(), } - // Test valid connection before returning - _, err := retTx.Do("PING") - - return retTx, err + return retTx, nil } type Tx struct { @@ -106,76 +103,47 @@ func createUser(userVals []string) (*models.User, error) { if len(userVals) != 7 { return nil, ErrCreateUser } - ID, err := strconv.ParseUint(userVals[0], 10, 64) - if err != nil { - return nil, err + var user models.User + convErrors := make([]error, 7) + user.ID, convErrors[0] = strconv.ParseUint(userVals[0], 10, 64) + user.Passkey = userVals[1] + user.UpMultiplier, convErrors[2] = strconv.ParseFloat(userVals[2], 64) + user.DownMultiplier, convErrors[3] = strconv.ParseFloat(userVals[3], 64) + user.Slots, convErrors[4] = strconv.ParseInt(userVals[4], 10, 64) + user.SlotsUsed, convErrors[5] = strconv.ParseInt(userVals[5], 10, 64) + user.Snatches, convErrors[6] = strconv.ParseUint(userVals[6], 10, 64) + + for i := 0; i < 7; i++ { + if convErrors[i] != nil { + return nil, convErrors[i] + } } - Passkey := userVals[1] - UpMultiplier, err := strconv.ParseFloat(userVals[2], 64) - if err != nil { - return nil, err - } - DownMultiplier, err := strconv.ParseFloat(userVals[3], 64) - if err != nil { - return nil, err - } - Slots, err := strconv.ParseInt(userVals[4], 10, 64) - if err != nil { - return nil, err - } - SlotsUsed, err := strconv.ParseInt(userVals[5], 10, 64) - if err != nil { - return nil, err - } - Snatches, err := strconv.ParseUint(userVals[6], 10, 64) - if err != nil { - return nil, err - } - return &models.User{ID: ID, Passkey: Passkey, UpMultiplier: UpMultiplier, - DownMultiplier: DownMultiplier, Slots: Slots, SlotsUsed: SlotsUsed, Snatches: uint(Snatches)}, nil + return &user, nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { if len(torrentVals) != 7 { return nil, ErrCreateTorrent } - ID, err := strconv.ParseUint(torrentVals[0], 10, 64) - if err != nil { - return nil, err - } - Infohash := torrentVals[1] - Active, err := strconv.ParseBool(torrentVals[2]) - if err != nil { - return nil, err - } - Snatches, err := strconv.ParseUint(torrentVals[3], 10, 32) - if err != nil { - return nil, err - } - UpMultiplier, err := strconv.ParseFloat(torrentVals[4], 64) - if err != nil { - return nil, err - } - DownMultiplier, err := strconv.ParseFloat(torrentVals[5], 64) - if err != nil { - return nil, err - } - LastAction, err := strconv.ParseInt(torrentVals[6], 10, 64) - if err != nil { - return nil, err - } - seeders, err := tx.getPeers(ID, SeederPrefix) - if err != nil { - return nil, err - } - leechers, err := tx.getPeers(ID, LeecherPrefix) - if err != nil { - return nil, err - } + var torrent models.Torrent + convErrors := make([]error, 9) + torrent.ID, convErrors[0] = strconv.ParseUint(torrentVals[0], 10, 64) + torrent.Infohash = torrentVals[1] + torrent.Active, convErrors[2] = strconv.ParseBool(torrentVals[2]) + torrent.Snatches, convErrors[3] = strconv.ParseUint(torrentVals[3], 10, 32) + torrent.UpMultiplier, convErrors[4] = strconv.ParseFloat(torrentVals[4], 64) + torrent.DownMultiplier, convErrors[5] = strconv.ParseFloat(torrentVals[5], 64) + torrent.LastAction, convErrors[6] = strconv.ParseInt(torrentVals[6], 10, 64) + torrent.Seeders, convErrors[7] = tx.getPeers(torrent.ID, SeedersPrefix) + torrent.Leechers, convErrors[8] = tx.getPeers(torrent.ID, LeechersPrefix) - return &models.Torrent{ID: ID, Infohash: Infohash, Active: Active, Seeders: seeders, Leechers: leechers, - Snatches: uint(Snatches), UpMultiplier: UpMultiplier, DownMultiplier: DownMultiplier, LastAction: LastAction}, nil + for i := 0; i < 9; i++ { + if convErrors[i] != nil { + return nil, convErrors[i] + } + } + return &torrent, nil } // The peer hashkey relies on the combination of peerID, userID, and torrentID being unique @@ -210,7 +178,7 @@ func (tx *Tx) removePeer(peer *models.Peer, peerTypePrefix string) error { return nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { hashKey := tx.conf.Prefix + getPeerHashKey(&peer) @@ -218,7 +186,7 @@ func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTy if err != nil { return err } - delete(peers, peer.ID) + delete(peers, models.PeerMapKey(&peer)) } // Will only delete the set if all the peer deletions were successful setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) @@ -238,7 +206,7 @@ func getPeerSetKey(typePrefix string, peer *models.Peer) string { return typePrefix + strconv.FormatUint(peer.TorrentID, 36) } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) addPeers(peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { setKey := tx.conf.Prefix + getPeerSetKey(peerTypePrefix, &peer) @@ -291,7 +259,7 @@ func createPeer(peerVals []string) (*models.Peer, error) { } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[string]models.Peer, err error) { peers = make(map[string]models.Peer) setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) @@ -318,7 +286,7 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin return } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) AddTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("HMSET", hashkey, @@ -333,18 +301,18 @@ func (tx *Tx) AddTorrent(t *models.Torrent) error { return err } - err = tx.addPeers(t.Seeders, SeederPrefix) + err = tx.addPeers(t.Seeders, SeedersPrefix) if err != nil { return err } - err = tx.addPeers(t.Leechers, LeecherPrefix) + err = tx.addPeers(t.Leechers, LeechersPrefix) if err != nil { return err } return nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) RemoveTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("DEL", hashkey) @@ -352,11 +320,11 @@ func (tx *Tx) RemoveTorrent(t *models.Torrent) error { return err } // Remove seeders and leechers as well - err = tx.removePeers(t.ID, t.Seeders, SeederPrefix) + err = tx.removePeers(t.ID, t.Seeders, SeedersPrefix) if err != nil { return err } - err = tx.removePeers(t.ID, t.Leechers, LeecherPrefix) + err = tx.removePeers(t.ID, t.Leechers, LeechersPrefix) if err != nil { return err } @@ -403,7 +371,7 @@ func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { return foundUser, true, nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { hashkey := tx.conf.Prefix + TorrentPrefix + infohash torrentStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) @@ -437,7 +405,7 @@ func (tx *Tx) UnWhitelistClient(peerID string) error { return err } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { torrentKey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash @@ -445,14 +413,14 @@ func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { if err != nil { return err } - torrent.Snatches = uint(snatchCount) + torrent.Snatches = uint64(snatchCount) userKey := tx.conf.Prefix + UserPrefix + user.Passkey snatchCount, err = redis.Int(tx.Do("HINCRBY", userKey, "snatches", 1)) if err != nil { return err } - user.Snatches = uint(snatchCount) + user.Snatches = uint64(snatchCount) return nil } @@ -470,7 +438,7 @@ func (tx *Tx) MarkActive(torrent *models.Torrent) error { return nil } -func (tx *Tx) MarkInActive(torrent *models.Torrent) error { +func (tx *Tx) MarkInactive(torrent *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash activeExists, err := redis.Int(tx.Do("HSET", hashkey, "active", false)) if err != nil { @@ -479,14 +447,19 @@ func (tx *Tx) MarkInActive(torrent *models.Torrent) error { torrent.Active = false // HSET returns 1 if hash didn't exist before if activeExists == 1 { + // Clean-up incomplete torrent + _, err = tx.Do("DEL", hashkey) + if err != nil { + return err + } return ErrMarkActive } return nil } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { - setKey := tx.conf.Prefix + LeecherPrefix + strconv.FormatUint(torrent.ID, 36) + setKey := tx.conf.Prefix + LeechersPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) if err != nil { return err @@ -514,7 +487,7 @@ func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { } func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { - err := tx.removePeer(p, LeecherPrefix) + err := tx.removePeer(p, LeechersPrefix) if err != nil { return err } @@ -524,8 +497,8 @@ func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error { torrentIdKey := strconv.FormatUint(torrent.ID, 36) - seederSetKey := tx.conf.Prefix + SeederPrefix + torrentIdKey - leecherSetKey := tx.conf.Prefix + LeecherPrefix + torrentIdKey + seederSetKey := tx.conf.Prefix + SeedersPrefix + torrentIdKey + leecherSetKey := tx.conf.Prefix + LeechersPrefix + torrentIdKey _, err := tx.Do("SMOVE", leecherSetKey, seederSetKey, getPeerHashKey(peer)) if err != nil { @@ -538,9 +511,9 @@ func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error return err } -// This is a mulple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { - setKey := tx.conf.Prefix + SeederPrefix + strconv.FormatUint(torrent.ID, 36) + setKey := tx.conf.Prefix + SeedersPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) if err != nil { return err @@ -566,7 +539,7 @@ func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { } func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { - err := tx.removePeer(p, SeederPrefix) + err := tx.removePeer(p, SeedersPrefix) if err != nil { return err } diff --git a/cache/redis/redis_bench_test.go b/cache/redis/redis_bench_test.go index 61bd0da..59cfe00 100644 --- a/cache/redis/redis_bench_test.go +++ b/cache/redis/redis_bench_test.go @@ -15,13 +15,13 @@ func BenchmarkSuccessfulFindUser(b *testing.B) { b.StopTimer() tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if !found { b.Error("user not found", testUser) } @@ -40,7 +40,7 @@ func BenchmarkFailedFindUser(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { _, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if found { b.Error("user not found", testUser) } @@ -52,12 +52,12 @@ func BenchmarkSuccessfulFindTorrent(b *testing.B) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if !found { b.Error("torrent not found", testTorrent) } @@ -76,7 +76,7 @@ func BenchmarkFailFindTorrent(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if found { b.Error("torrent found", foundTorrent) } @@ -87,12 +87,12 @@ func BenchmarkSuccessfulClientWhitelisted(b *testing.B) { b.StopTimer() tx := createTestTx() testPeerID := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID)) + panicOnErr(tx.WhitelistClient(testPeerID)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { found, err := tx.ClientWhitelisted(testPeerID) - panicErrNil(err) + panicOnErr(err) if !found { b.Error("peerID not found", testPeerID) } @@ -107,7 +107,7 @@ func BenchmarkFailClientWhitelisted(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { found, err := tx.ClientWhitelisted(testPeerID2) - panicErrNil(err) + panicOnErr(err) if found { b.Error("peerID found", testPeerID2) } @@ -119,12 +119,12 @@ func BenchmarkRecordSnatch(b *testing.B) { tx := createTestTx() testTorrent := createTestTorrent() testUser := createTestUser() - panicErrNil(tx.AddTorrent(testTorrent)) - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddUser(testUser)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { - panicErrNil(tx.RecordSnatch(testUser, testTorrent)) + panicOnErr(tx.RecordSnatch(testUser, testTorrent)) } } @@ -133,11 +133,11 @@ func BenchmarkMarkActive(b *testing.B) { tx := createTestTx() testTorrent := createTestTorrent() testTorrent.Active = false - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { - panicErrNil(tx.MarkActive(testTorrent)) + panicOnErr(tx.MarkActive(testTorrent)) } } @@ -145,7 +145,7 @@ func BenchmarkAddSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { @@ -153,7 +153,7 @@ func BenchmarkAddSeeder(b *testing.B) { testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) b.StartTimer() - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) } } @@ -161,7 +161,7 @@ func BenchmarkRemoveSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) b.StartTimer() @@ -170,7 +170,7 @@ func BenchmarkRemoveSeeder(b *testing.B) { tx.AddSeeder(testTorrent, testSeeder) b.StartTimer() - panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + panicOnErr(tx.RemoveSeeder(testTorrent, testSeeder)) } } @@ -178,9 +178,9 @@ func BenchmarkSetSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) r := rand.New(rand.NewSource(time.Now().UnixNano())) b.StartTimer() @@ -197,11 +197,11 @@ func BenchmarkIncrementSlots(b *testing.B) { b.StopTimer() tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { - panicErrNil(tx.IncrementSlots(testUser)) + panicOnErr(tx.IncrementSlots(testUser)) } } @@ -209,17 +209,17 @@ func BenchmarkLeecherFinished(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { b.StopTimer() testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) testLeecher.Left = 0 b.StartTimer() - panicErrNil(tx.LeecherFinished(testTorrent, testLeecher)) + panicOnErr(tx.LeecherFinished(testTorrent, testLeecher)) } } @@ -228,18 +228,18 @@ func BenchmarkRemoveLeecherAddSeeder(b *testing.B) { b.StopTimer() tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) b.StartTimer() for bCount := 0; bCount < b.N; bCount++ { b.StopTimer() testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) testLeecher.Left = 0 b.StartTimer() - panicErrNil(tx.RemoveLeecher(testTorrent, testLeecher)) - panicErrNil(tx.AddSeeder(testTorrent, testLeecher)) + panicOnErr(tx.RemoveLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddSeeder(testTorrent, testLeecher)) } } diff --git a/cache/redis/redis_test.go b/cache/redis/redis_test.go index d670bb0..f907a0d 100644 --- a/cache/redis/redis_test.go +++ b/cache/redis/redis_test.go @@ -76,7 +76,7 @@ func createTestPasskey() string { return string(uuid) } -func panicErrNil(err error) { +func panicOnErr(err error) { if err != nil { fmt.Println(err) panic(err) @@ -86,7 +86,7 @@ func panicErrNil(err error) { func createTestRedisTx() *Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) conf := &testConfig.Cache - panicErrNil(err) + panicOnErr(err) testPool := &Pool{ conf: conf, @@ -103,11 +103,11 @@ func createTestRedisTx() *Tx { done: false, Conn: testPool.pool.Get(), } - panicErrNil(err) + panicOnErr(err) // Test connection before returning _, err = txObj.Do("PING") - panicErrNil(err) + panicOnErr(err) return txObj } @@ -144,53 +144,18 @@ func createTestTorrent() *models.Torrent { return &testTorrent } -func comparePeers(lhPeers map[string]models.Peer, rhPeers map[string]models.Peer) bool { - if len(lhPeers) != len(rhPeers) { - return false - } - for rhKey, rhValue := range rhPeers { - lhValue, lhExists := lhPeers[rhKey] - if !lhExists || lhValue != rhValue { - return false - } - } - for lhKey, lhValue := range lhPeers { - rhValue, rhExists := rhPeers[lhKey] - if !rhExists || rhValue != lhValue { - return false - } - } - return true -} - -func torrentsEqual(lhTorrent *models.Torrent, rhTorrent *models.Torrent) bool { - fieldsEqual := lhTorrent.Infohash == rhTorrent.Infohash && - lhTorrent.ID == rhTorrent.ID && - lhTorrent.Active == rhTorrent.Active && - lhTorrent.Snatches == rhTorrent.Snatches && - lhTorrent.UpMultiplier == rhTorrent.UpMultiplier && - lhTorrent.DownMultiplier == rhTorrent.DownMultiplier && - lhTorrent.LastAction == rhTorrent.LastAction - - if !fieldsEqual { - return false - } - - return comparePeers(lhTorrent.Seeders, rhTorrent.Seeders) && comparePeers(lhTorrent.Leechers, rhTorrent.Leechers) -} - func TestValidPeers(t *testing.T) { testTx := createTestRedisTx() testTorrentID := createTestTorrentID() testPeers := createTestPeers(testTorrentID, 3) - panicErrNil(testTx.addPeers(testPeers, "test:")) + panicOnErr(testTx.addPeers(testPeers, "test:")) peerMap, err := testTx.getPeers(testTorrentID, "test:") - panicErrNil(err) + panicOnErr(err) if len(peerMap) != len(testPeers) { t.Error("Num Peers not equal ", len(peerMap), len(testPeers)) } - panicErrNil(testTx.removePeers(testTorrentID, testPeers, "test:")) + panicOnErr(testTx.removePeers(testTorrentID, testPeers, "test:")) } func TestInvalidPeers(t *testing.T) { @@ -200,17 +165,20 @@ func TestInvalidPeers(t *testing.T) { tempPeer := createTestPeer(createTestUserID(), testTorrentID) testPeers[models.PeerMapKey(tempPeer)] = *tempPeer - panicErrNil(testTx.addPeers(testPeers, "test:")) + panicOnErr(testTx.addPeers(testPeers, "test:")) // Imitate a peer being removed during get hashKey := testTx.conf.Prefix + getPeerHashKey(tempPeer) _, err := testTx.Do("DEL", hashKey) - panicErrNil(err) + panicOnErr(err) peerMap, err := testTx.getPeers(testTorrentID, "test:") - panicErrNil(err) + panicOnErr(err) // Expect 1 less peer due to delete if len(peerMap) != len(testPeers)-1 { - t.Error("Num Peers not equal ", len(peerMap), len(testPeers)) + t.Error("Num Peers not equal ", len(peerMap), len(testPeers)-1) + } + panicOnErr(testTx.removePeers(testTorrentID, testPeers, "test:")) + if len(testPeers) != 0 { + t.Errorf("All peers not removed, %d peers remain!", len(testPeers)) } - panicErrNil(testTx.removePeers(testTorrentID, testPeers, "test:")) } diff --git a/cache/redis/tx_test.go b/cache/redis/tx_test.go index f107f1e..4d39b47 100644 --- a/cache/redis/tx_test.go +++ b/cache/redis/tx_test.go @@ -7,6 +7,7 @@ package redis import ( "math/rand" "os" + "reflect" "testing" "time" @@ -17,14 +18,14 @@ import ( func createTestTx() cache.Tx { testConfig, err := config.Open(os.Getenv("TESTCONFIGPATH")) - panicErrNil(err) + panicOnErr(err) conf := &testConfig.Cache testPool, err := cache.Open(conf) - panicErrNil(err) + panicOnErr(err) txObj, err := testPool.Get() - panicErrNil(err) + panicOnErr(err) return txObj } @@ -33,9 +34,9 @@ func TestFindUserSuccess(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("user not found", testUser) } @@ -49,7 +50,7 @@ func TestFindUserFail(t *testing.T) { testUser := createTestUser() foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if found { t.Error("user found", foundUser) } @@ -59,11 +60,11 @@ func TestRemoveUser(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) err := tx.RemoveUser(testUser) - panicErrNil(err) + panicOnErr(err) foundUser, found, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if found { t.Error("removed user found", foundUser) } @@ -72,14 +73,14 @@ func TestRemoveUser(t *testing.T) { func TestFindTorrentSuccess(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("torrent not found", testTorrent) } - if !torrentsEqual(foundTorrent, testTorrent) { + if !reflect.DeepEqual(foundTorrent, testTorrent) { t.Error("found torrent mismatch", foundTorrent, testTorrent) } } @@ -89,7 +90,7 @@ func TestFindTorrentFail(t *testing.T) { testTorrent := createTestTorrent() foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if found { t.Error("torrent found", foundTorrent) } @@ -98,11 +99,11 @@ func TestFindTorrentFail(t *testing.T) { func TestRemoveTorrent(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) - panicErrNil(tx.RemoveTorrent(testTorrent)) + panicOnErr(tx.RemoveTorrent(testTorrent)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if found { t.Error("removed torrent found", foundTorrent) } @@ -112,9 +113,9 @@ func TestClientWhitelistSuccess(t *testing.T) { tx := createTestTx() testPeerID := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID)) + panicOnErr(tx.WhitelistClient(testPeerID)) found, err := tx.ClientWhitelisted(testPeerID) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("peerID not found", testPeerID) } @@ -125,7 +126,7 @@ func TestClientWhitelistFail(t *testing.T) { testPeerID2 := "TIX0192" found, err := tx.ClientWhitelisted(testPeerID2) - panicErrNil(err) + panicOnErr(err) if found { t.Error("peerID found", testPeerID2) } @@ -135,18 +136,18 @@ func TestRecordSnatch(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() testUser := createTestUser() - panicErrNil(tx.AddTorrent(testTorrent)) - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddUser(testUser)) userSnatches := testUser.Snatches torrentSnatches := testTorrent.Snatches - panicErrNil(tx.RecordSnatch(testUser, testTorrent)) + panicOnErr(tx.RecordSnatch(testUser, testTorrent)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundUser, _, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if testUser.Snatches != userSnatches+1 { t.Error("snatch not recorded to local user", testUser.Snatches, userSnatches+1) @@ -166,11 +167,11 @@ func TestMarkActive(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() testTorrent.Active = false - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) - panicErrNil(tx.MarkActive(testTorrent)) + panicOnErr(tx.MarkActive(testTorrent)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if foundTorrent.Active != true { t.Error("cached torrent not activated") @@ -183,11 +184,11 @@ func TestMarkActive(t *testing.T) { func TestClientWhitelistRemove(t *testing.T) { tx := createTestTx() testPeerID := "-lt0D30-" - panicErrNil(tx.WhitelistClient(testPeerID)) - panicErrNil(tx.UnWhitelistClient(testPeerID)) + panicOnErr(tx.WhitelistClient(testPeerID)) + panicOnErr(tx.UnWhitelistClient(testPeerID)) found, err := tx.ClientWhitelisted(testPeerID) - panicErrNil(err) + panicOnErr(err) if found { t.Error("removed peerID found", testPeerID) } @@ -196,12 +197,12 @@ func TestClientWhitelistRemove(t *testing.T) { func TestAddSeeder(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, found := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if found && foundSeeder != *testSeeder { t.Error("seeder not added to cache", testSeeder) @@ -215,12 +216,12 @@ func TestAddSeeder(t *testing.T) { func TestAddLeecher(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if found && foundLeecher != *testLeecher { t.Error("leecher not added to cache", testLeecher) @@ -234,18 +235,18 @@ func TestAddLeecher(t *testing.T) { func TestRemoveSeeder(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) - panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + panicOnErr(tx.RemoveSeeder(testTorrent, testSeeder)) foundSeeder, found := testTorrent.Seeders[models.PeerMapKey(testSeeder)] if found || foundSeeder == *testSeeder { t.Error("seeder not removed from local", foundSeeder) } foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, found = foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if found || foundSeeder == *testSeeder { t.Error("seeder not removed from cache", foundSeeder, *testSeeder) @@ -255,13 +256,13 @@ func TestRemoveSeeder(t *testing.T) { func TestRemoveLeecher(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) - panicErrNil(tx.RemoveLeecher(testTorrent, testLeecher)) + panicOnErr(tx.RemoveLeecher(testTorrent, testLeecher)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if found || foundLeecher == *testLeecher { t.Error("leecher not removed from cache", foundLeecher, *testLeecher) @@ -275,17 +276,17 @@ func TestRemoveLeecher(t *testing.T) { func TestSetSeeder(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) r := rand.New(rand.NewSource(time.Now().UnixNano())) testSeeder.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + panicOnErr(tx.SetSeeder(testTorrent, testSeeder)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if foundSeeder != *testSeeder { t.Error("seeder not updated in cache", foundSeeder, *testSeeder) @@ -299,16 +300,16 @@ func TestSetSeeder(t *testing.T) { func TestSetLeecher(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) r := rand.New(rand.NewSource(time.Now().UnixNano())) testLeecher.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetLeecher(testTorrent, testLeecher)) + panicOnErr(tx.SetLeecher(testTorrent, testLeecher)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, _ := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if foundLeecher != *testLeecher { t.Error("leecher not updated in cache", testLeecher) @@ -322,12 +323,12 @@ func TestSetLeecher(t *testing.T) { func TestIncrementSlots(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) numSlots := testUser.Slots - panicErrNil(tx.IncrementSlots(testUser)) + panicOnErr(tx.IncrementSlots(testUser)) foundUser, _, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if foundUser.Slots != numSlots+1 { t.Error("cached slots not incremented") @@ -340,12 +341,12 @@ func TestIncrementSlots(t *testing.T) { func TestDecrementSlots(t *testing.T) { tx := createTestTx() testUser := createTestUser() - panicErrNil(tx.AddUser(testUser)) + panicOnErr(tx.AddUser(testUser)) numSlots := testUser.Slots - panicErrNil(tx.DecrementSlots(testUser)) + panicOnErr(tx.DecrementSlots(testUser)) foundUser, _, err := tx.FindUser(testUser.Passkey) - panicErrNil(err) + panicOnErr(err) if foundUser.Slots != numSlots-1 { t.Error("cached slots not incremented") @@ -358,15 +359,15 @@ func TestDecrementSlots(t *testing.T) { func TestLeecherFinished(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) testLeecher.Left = 0 - panicErrNil(tx.LeecherFinished(testTorrent, testLeecher)) + panicOnErr(tx.LeecherFinished(testTorrent, testLeecher)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testLeecher)] if foundSeeder != *testLeecher { t.Error("seeder not added to cache", foundSeeder, *testLeecher) @@ -390,17 +391,17 @@ func TestUpdatePeer(t *testing.T) { tx := createTestTx() testTorrent := createTestTorrent() testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddTorrent(testTorrent)) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) // Update a seeder, set it, then check to make sure it updated r := rand.New(rand.NewSource(time.Now().UnixNano())) testSeeder.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + panicOnErr(tx.SetSeeder(testTorrent, testSeeder)) - panicErrNil(tx.RemoveSeeder(testTorrent, testSeeder)) + panicOnErr(tx.RemoveSeeder(testTorrent, testSeeder)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) if seeder, exists := foundTorrent.Seeders[models.PeerMapKey(testSeeder)]; exists { t.Error("seeder not removed from cache", seeder) } @@ -417,16 +418,16 @@ func TestParallelFindUser(t *testing.T) { tx := createTestTx() testUserSuccess := createTestUser() testUserFail := createTestUser() - panicErrNil(tx.AddUser(testUserSuccess)) + panicOnErr(tx.AddUser(testUserSuccess)) for i := 0; i < 10; i++ { foundUser, found, err := tx.FindUser(testUserFail.Passkey) - panicErrNil(err) + panicOnErr(err) if found { t.Error("user found", foundUser) } foundUser, found, err = tx.FindUser(testUserSuccess.Passkey) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("user not found", testUserSuccess) } @@ -444,19 +445,19 @@ func TestParallelFindTorrent(t *testing.T) { tx := createTestTx() testTorrentSuccess := createTestTorrent() testTorrentFail := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrentSuccess)) + panicOnErr(tx.AddTorrent(testTorrentSuccess)) for i := 0; i < 10; i++ { foundTorrent, found, err := tx.FindTorrent(testTorrentSuccess.Infohash) - panicErrNil(err) + panicOnErr(err) if !found { t.Error("torrent not found", testTorrentSuccess) } - if !torrentsEqual(foundTorrent, testTorrentSuccess) { + if !reflect.DeepEqual(foundTorrent, testTorrentSuccess) { t.Error("found torrent mismatch", foundTorrent, testTorrentSuccess) } foundTorrent, found, err = tx.FindTorrent(testTorrentFail.Infohash) - panicErrNil(err) + panicOnErr(err) if found { t.Error("torrent found", foundTorrent) } @@ -470,18 +471,18 @@ func TestParallelSetSeeder(t *testing.T) { } tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) testSeeder := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddSeeder(testTorrent, testSeeder)) + panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) r := rand.New(rand.NewSource(time.Now().UnixNano())) for i := 0; i < 10; i++ { testSeeder.Uploaded += uint64(r.Int63()) - panicErrNil(tx.SetSeeder(testTorrent, testSeeder)) + panicOnErr(tx.SetSeeder(testTorrent, testSeeder)) foundTorrent, _, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundSeeder, _ := foundTorrent.Seeders[models.PeerMapKey(testSeeder)] if foundSeeder != *testSeeder { t.Error("seeder not updated in cache", foundSeeder, *testSeeder) @@ -500,15 +501,15 @@ func TestParallelAddLeecher(t *testing.T) { } tx := createTestTx() testTorrent := createTestTorrent() - panicErrNil(tx.AddTorrent(testTorrent)) + panicOnErr(tx.AddTorrent(testTorrent)) for i := 0; i < 10; i++ { testLeecher := createTestPeer(createTestUserID(), testTorrent.ID) - panicErrNil(tx.AddLeecher(testTorrent, testLeecher)) + panicOnErr(tx.AddLeecher(testTorrent, testLeecher)) foundTorrent, found, err := tx.FindTorrent(testTorrent.Infohash) - panicErrNil(err) + panicOnErr(err) foundLeecher, found := foundTorrent.Leechers[models.PeerMapKey(testLeecher)] if found && foundLeecher != *testLeecher { t.Error("leecher not added to cache", testLeecher) diff --git a/models/models.go b/models/models.go index 1162321..5599c94 100644 --- a/models/models.go +++ b/models/models.go @@ -34,7 +34,7 @@ type Torrent struct { Seeders map[string]Peer `json:"seeders"` Leechers map[string]Peer `json:"leechers"` - Snatches uint `json:"snatches"` + Snatches uint64 `json:"snatches"` UpMultiplier float64 `json:"up_multiplier"` DownMultiplier float64 `json:"down_multiplier"` LastAction int64 `json:"last_action"` @@ -48,5 +48,5 @@ type User struct { DownMultiplier float64 `json:"down_multiplier"` Slots int64 `json:"slots"` SlotsUsed int64 `json:"slots_used"` - Snatches uint `json:"snatches"` + Snatches uint64 `json:"snatches"` } From b7dc80e4a310102963e1edf6749b6ec684721040 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Fri, 27 Sep 2013 22:08:28 -0400 Subject: [PATCH 7/9] Added documentation to redis package --- cache/redis/redis.go | 218 ++++++++++++++++++++++++++++++++----------- 1 file changed, 165 insertions(+), 53 deletions(-) diff --git a/cache/redis/redis.go b/cache/redis/redis.go index af62bd7..64eb885 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -4,13 +4,23 @@ // Package redis implements the storage interface for a BitTorrent tracker. // -// The client whitelist is represented as a set with the key name "whitelist" -// with an optional prefix. Torrents and users are represented as hashes. -// Torrents' keys are named "torrent:" with an optional prefix. -// Users' keys are named "user:" with an optional prefix. The -// seeders and leechers attributes of torrent hashes are strings that represent -// the key for those hashes within redis. This is done because redis cannot -// nest their hash data type. +// This interface is configured by a config.DataStore. +// To get a handle to this interface, call New on the initialized driver and +// then Get() on returned the cache.Pool. +// +// Torrents, Users, and Peers are all stored in Redis hash types. All Redis +// keys can have an optional prefix specified during configuration. +// The relationship between Torrents and Peers is a Redis set that holds +// the peers' keys. There are two sets per torrent, one for seeders and +// one for leechers. The Redis sets are keyed by type and the torrent's ID. +// +// The whitelist is a Redis set with the key "whitelist" that holds client IDs. +// Operations on the whitelist do not parse the client ID from a peer ID. +// +// Some functions in this interface are not atomic. The data being modified may +// change while the function is executing. This will not cause the function to +// return an error; instead the function will complete and return valid, stale +// data. package redis import ( @@ -40,6 +50,7 @@ var ( type driver struct{} +// New creates and returns a cache.Pool. func (d *driver) New(conf *config.DataStore) cache.Pool { return &Pool{ conf: conf, @@ -52,6 +63,7 @@ func (d *driver) New(conf *config.DataStore) cache.Pool { } } +// makeDialFunc configures and returns a new redis.Dial struct using the specified configuration. func makeDialFunc(conf *config.DataStore) func() (redis.Conn, error) { return func() (conn redis.Conn, err error) { conn, err = redis.Dial(conf.Network, conf.Host+":"+conf.Port) @@ -62,6 +74,7 @@ func makeDialFunc(conf *config.DataStore) func() (redis.Conn, error) { } } +// testOnBorrow pings the Redis instance func testOnBorrow(c redis.Conn, t time.Time) error { _, err := c.Do("PING") return err @@ -99,6 +112,20 @@ func (tx *Tx) close() { tx.Conn.Close() } +// createUser takes a slice of length 7 and returns a pointer to a new models.User or an error. +// This function is used to create a user from a Redis hash response. +// The 7 strings in the slice must be in the specified order. +// +// User.ID +// User.Passkey +// User.UpMultiplier +// User.DownMultiplier +// User.Slots +// User.SlotsUsed +// User.Snatches +// +// If the strings cannot be converted to the correct type, +// createUser will return a nil user and the conversion error. func createUser(userVals []string) (*models.User, error) { if len(userVals) != 7 { return nil, ErrCreateUser @@ -121,7 +148,22 @@ func createUser(userVals []string) (*models.User, error) { return &user, nil } -// This is a multiple action command, it's not internally atomic +// createTorrent takes a slice of length 7 and returns a pointer to a new models.Torrent or an error. +// This function can be used to create a torrent from a Redis hash response. +// The 7 hash fields must be in the specified order. +// +// torrent.ID +// torrent.Infohash +// torrent.Active +// torrent.Snatches +// torrent.UpMultiplier +// torrent.DownMultiplier +// torrent.LastAction +// +// This function calls multiple redis commands, it's not internally atomic. +// After converting the torrent's ID, the seeders and leechers are populated by getPeers +// If the strings cannot be converted to the correct type, +// createTorrent will return a nil user and the conversion error. func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { if len(torrentVals) != 7 { return nil, ErrCreateTorrent @@ -146,7 +188,9 @@ func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { return &torrent, nil } -// The peer hashkey relies on the combination of peerID, userID, and torrentID being unique +// setPeer writes or overwrites peer information. +// The hash fields are sent in a specific order +// so that they can be unpacked correctly func (tx *Tx) setPeer(peer *models.Peer) error { hashKey := tx.conf.Prefix + getPeerHashKey(peer) _, err := tx.Do("HMSET", hashKey, @@ -160,13 +204,13 @@ func (tx *Tx) setPeer(peer *models.Peer) error { "left", peer.Left, "last_announce", peer.LastAnnounce) - if err != nil { - return err - } - return nil + return err } -// Will not return an error if the peer doesn't exist +// removePeer removes the given peer from the specified peer set (seeder or leecher), +// and removes the peer information. +// This function calls multiple redis commands, it's not internally atomic. +// This function will not return an error if the peer to remove doesn't exist. func (tx *Tx) removePeer(peer *models.Peer, peerTypePrefix string) error { setKey := tx.conf.Prefix + getPeerSetKey(peerTypePrefix, peer) _, err := tx.Do("SREM", setKey, getPeerHashKey(peer)) @@ -178,7 +222,11 @@ func (tx *Tx) removePeer(peer *models.Peer, peerTypePrefix string) error { return nil } -// This is a multiple action command, it's not internally atomic +// removePeers removes all peers from specified peer set (seeders or leechers), +// removes the peer information, and then removes the associated peer from the given map. +// This function will not return an error if the peer to remove doesn't exist. +// This function will only delete the peer set if all the individual peer deletions were successful +// This function calls multiple redis commands, it's not internally atomic. func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { hashKey := tx.conf.Prefix + getPeerHashKey(&peer) @@ -188,25 +236,31 @@ func (tx *Tx) removePeers(torrentID uint64, peers map[string]models.Peer, peerTy } delete(peers, models.PeerMapKey(&peer)) } - // Will only delete the set if all the peer deletions were successful setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) _, err := tx.Do("DEL", setKey) if err != nil { return err } - return nil } +// getPeerHashKey returns a string with the peer.ID, encoded peer.UserID, and encoded peer.TorrentID, +// concatenated and delimited by colons +// This key corresponds to a Redis hash type with fields containing a peer's data. +// The peer hashkey relies on the combination of peerID, userID, and torrentID being unique. func getPeerHashKey(peer *models.Peer) string { return peer.ID + ":" + strconv.FormatUint(peer.UserID, 36) + ":" + strconv.FormatUint(peer.TorrentID, 36) } +// getPeerSetKey returns a string that is the peer's encoded torrentID appended to the typePrefix +// This key corresponds to a torrent's pool of leechers or seeders func getPeerSetKey(typePrefix string, peer *models.Peer) string { return typePrefix + strconv.FormatUint(peer.TorrentID, 36) } -// This is a multiple action command, it's not internally atomic +// addPeers adds each peer's key to the specified peer set and saves the peer's information. +// This function will not return an error if the peer already exists in the set. +// This function calls multiple redis commands, it's not internally atomic. func (tx *Tx) addPeers(peers map[string]models.Peer, peerTypePrefix string) error { for _, peer := range peers { setKey := tx.conf.Prefix + getPeerSetKey(peerTypePrefix, &peer) @@ -219,46 +273,47 @@ func (tx *Tx) addPeers(peers map[string]models.Peer, peerTypePrefix string) erro return nil } +// createPeer takes a slice of length 9 and returns a pointer to a new models.Peer or an error. +// This function is used to create a peer from a Redis hash response. +// The 9 strings in the slice must be in the specified order. +// +// peer.ID +// peer.UserID +// peer.TorrentID +// peer.IP +// peer.Port +// peer.Uploaded +// peer.Downloaded +// peer.Left +// peer.LastAnnounce +// +// If the strings cannot be converted to the correct type, +// This function will return a nil peer and the conversion error. func createPeer(peerVals []string) (*models.Peer, error) { if len(peerVals) != 9 { return nil, ErrCreatePeer } - ID := peerVals[0] - UserID, err := strconv.ParseUint(peerVals[1], 10, 64) - if err != nil { - return nil, err - } - TorrentID, err := strconv.ParseUint(peerVals[2], 10, 64) - if err != nil { - return nil, err - } - IP := peerVals[3] + var peer models.Peer + convErrors := make([]error, 9) + peer.ID = peerVals[0] + peer.UserID, convErrors[1] = strconv.ParseUint(peerVals[1], 10, 64) + peer.TorrentID, convErrors[2] = strconv.ParseUint(peerVals[2], 10, 64) + peer.IP = peerVals[3] + peer.Port, convErrors[4] = strconv.ParseUint(peerVals[4], 10, 64) + peer.Uploaded, convErrors[5] = strconv.ParseUint(peerVals[5], 10, 64) + peer.Downloaded, convErrors[6] = strconv.ParseUint(peerVals[6], 10, 64) + peer.Left, convErrors[7] = strconv.ParseUint(peerVals[7], 10, 64) + peer.LastAnnounce, convErrors[8] = strconv.ParseInt(peerVals[8], 10, 64) - Port, err := strconv.ParseUint(peerVals[4], 10, 64) - if err != nil { - return nil, err + for i := 0; i < 9; i++ { + if convErrors[i] != nil { + return nil, convErrors[i] + } } - Uploaded, err := strconv.ParseUint(peerVals[5], 10, 64) - if err != nil { - return nil, err - } - Downloaded, err := strconv.ParseUint(peerVals[6], 10, 64) - if err != nil { - return nil, err - } - Left, err := strconv.ParseUint(peerVals[7], 10, 64) - if err != nil { - return nil, err - } - LastAnnounce, err := strconv.ParseInt(peerVals[8], 10, 64) - if err != nil { - return nil, err - } - return &models.Peer{ID: ID, UserID: UserID, TorrentID: TorrentID, IP: IP, Port: Port, - Uploaded: Uploaded, Downloaded: Downloaded, Left: Left, LastAnnounce: LastAnnounce}, nil - + return &peer, nil } +// getPeers returns a map of peers from a specified torrent's peer set(seeders or leechers). // This is a multiple action command, it's not internally atomic func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[string]models.Peer, err error) { peers = make(map[string]models.Peer) @@ -286,6 +341,9 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin return } +// AddTorrent writes/overwrites torrent information and saves peers from both peer sets. +// The hash fields are sent in a specific order +// so that they can be unpacked correctly // This is a multiple action command, it's not internally atomic func (tx *Tx) AddTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash @@ -312,7 +370,9 @@ func (tx *Tx) AddTorrent(t *models.Torrent) error { return nil } -// This is a multiple action command, it's not internally atomic +// RemoveTorrent deletes the torrent's Redis hash and then deletes all peers. +// This function will not return an error if the torrent has already been removed. +// This is a multiple action command, it's not internally atomic. func (tx *Tx) RemoveTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("DEL", hashkey) @@ -331,6 +391,9 @@ func (tx *Tx) RemoveTorrent(t *models.Torrent) error { return nil } +// AddUser writes/overwrites user information to a Redis hash. +// The hash fields are sent in a specific order +// so that they can be unpacked correctly func (tx *Tx) AddUser(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey _, err := tx.Do("HMSET", hashkey, @@ -347,6 +410,8 @@ func (tx *Tx) AddUser(u *models.User) error { return nil } +// RemoveUser removes the user's hash from Redis. +// This function does not return an error if the user doesn't exist. func (tx *Tx) RemoveUser(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey _, err := tx.Do("DEL", hashkey) @@ -356,8 +421,12 @@ func (tx *Tx) RemoveUser(u *models.User) error { return nil } +// FindUser returns true and a pointer to a new user struct, if the user exists +// or nil and false if the user doesn't exist. +// This function does not return an error if the torrent doesn't exist. func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { hashkey := tx.conf.Prefix + UserPrefix + passkey + // Consider using HGETALL instead of HVALS here for robustness userStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) if err != nil { return nil, false, err @@ -371,6 +440,8 @@ func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { return foundUser, true, nil } +// FindTorrent returns a pointer to a new torrent struct and true, if the torrent exists +// or nil and false if the torrent doesn't exist. // This is a multiple action command, it's not internally atomic func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { hashkey := tx.conf.Prefix + TorrentPrefix + infohash @@ -388,23 +459,32 @@ func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { return foundTorrent, true, nil } +// ClientWhitelisted returns true if the ClientID exists in the Client set. +// This function does not parse the client ID from the peer ID. +// The clientID must match exactly to a member of the set. func (tx *Tx) ClientWhitelisted(peerID string) (exists bool, err error) { key := tx.conf.Prefix + "whitelist" return redis.Bool(tx.Do("SISMEMBER", key, peerID)) } +// WhitelistClient adds a client ID to the client whitelist set. +// This function does not return an error if the client ID is already in the set. func (tx *Tx) WhitelistClient(peerID string) error { key := tx.conf.Prefix + "whitelist" _, err := tx.Do("SADD", key, peerID) return err } +// UnWhitelistClient removes a client ID from the client whitelist set +// This function does not return an error if the client ID is not in the set. func (tx *Tx) UnWhitelistClient(peerID string) error { key := tx.conf.Prefix + "whitelist" _, err := tx.Do("SREM", key, peerID) return err } +// RecordSnatch increments the snatch counter on the torrent and user by one. +// This modifies the arguments as well as the hash field in Redis. // This is a multiple action command, it's not internally atomic func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { @@ -424,6 +504,9 @@ func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { return nil } +// MarkActive sets the active field of the torrent to true. +// This modifies the argument as well as the hash field in Redis. +// This function will return ErrMarkActive if the torrent does not exist. func (tx *Tx) MarkActive(torrent *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash activeExists, err := redis.Int(tx.Do("HSET", hashkey, "active", true)) @@ -438,6 +521,9 @@ func (tx *Tx) MarkActive(torrent *models.Torrent) error { return nil } +// MarkInactive sets the active field of the torrent to false. +// This modifies the argument as well as the hash field in Redis. +// This function will return ErrMarkActive if the torrent does not exist. func (tx *Tx) MarkInactive(torrent *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash activeExists, err := redis.Int(tx.Do("HSET", hashkey, "active", false)) @@ -457,6 +543,9 @@ func (tx *Tx) MarkInactive(torrent *models.Torrent) error { return nil } +// AddLeecher adds a new peer to a torrent's leecher set. +// This modifies the torrent argument, as well as the torrent's set and peer's hash in Redis. +// This function does not return an error if the leecher already exists. // This is a multiple action command, it's not internally atomic func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { setKey := tx.conf.Prefix + LeechersPrefix + strconv.FormatUint(torrent.ID, 36) @@ -475,8 +564,10 @@ func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { return nil } -// Setting assumes it is already a leecher, and just needs to be updated -// Maybe eventually there will be a move from leecher to seeder method +// SetLeecher updates a torrent's leecher. +// This modifies the torrent argument, as well as the peer's hash in Redis. +// Setting assumes that the peer is already a leecher, and only needs to be updated. +// This function does not return an error if the leecher does not exist or is not in the torrent's leecher set. func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { err := tx.setPeer(p) if err != nil { @@ -486,6 +577,9 @@ func (tx *Tx) SetLeecher(t *models.Torrent, p *models.Peer) error { return nil } +// RemoveLeecher removes the given peer from a torrent's leecher set. +// This modifies the torrent argument, as well as the torrent's set and peer's hash in Redis. +// This function does not return an error if the peer doesn't exist, or is not in the set. func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { err := tx.removePeer(p, LeechersPrefix) if err != nil { @@ -495,6 +589,9 @@ func (tx *Tx) RemoveLeecher(t *models.Torrent, p *models.Peer) error { return nil } +// LeecherFinished moves a peer's hashkey from a torrent's leecher set to the seeder set and updates the peer. +// This modifies the torrent argument, as well as the torrent's set and peer's hash in Redis. +// This function does not return an error if the peer doesn't exist or is not in the torrent's leecher set. func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error { torrentIdKey := strconv.FormatUint(torrent.ID, 36) seederSetKey := tx.conf.Prefix + SeedersPrefix + torrentIdKey @@ -511,6 +608,9 @@ func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error return err } +// AddSeeder adds a new peer to a torrent's seeder set. +// This modifies the torrent argument, as well as the torrent's set and peer's hash in Redis. +// This function does not return an error if the seeder already exists. // This is a multiple action command, it's not internally atomic func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { setKey := tx.conf.Prefix + SeedersPrefix + strconv.FormatUint(torrent.ID, 36) @@ -529,6 +629,10 @@ func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { return nil } +// SetSeeder updates a torrent's seeder. +// This modifies the torrent argument, as well as the peer's hash in Redis. +// Setting assumes that the peer is already a seeder, and only needs to be updated. +// This function does not return an error if the seeder does not exist or is not in the torrent's seeder set. func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { err := tx.setPeer(p) if err != nil { @@ -538,6 +642,9 @@ func (tx *Tx) SetSeeder(t *models.Torrent, p *models.Peer) error { return nil } +// RemoveSeeder removes the given peer from a torrent's seeder set. +// This modifies the torrent argument, as well as the torrent's set and peer's hash in Redis. +// This function does not return an error if the peer doesn't exist, or is not in the set. func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { err := tx.removePeer(p, SeedersPrefix) if err != nil { @@ -547,6 +654,8 @@ func (tx *Tx) RemoveSeeder(t *models.Torrent, p *models.Peer) error { return nil } +// IncrementSlots increment a user's Slots by one. +// This function modifies the argument as well as the hash field in Redis. func (tx *Tx) IncrementSlots(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, "slots", 1)) @@ -557,6 +666,8 @@ func (tx *Tx) IncrementSlots(u *models.User) error { return nil } +// IncrementSlots increment a user's Slots by one. +// This function modifies the argument as well as the hash field in Redis. func (tx *Tx) DecrementSlots(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey slotCount, err := redis.Int(tx.Do("HINCRBY", hashkey, "slots", -1)) @@ -567,6 +678,7 @@ func (tx *Tx) DecrementSlots(u *models.User) error { return nil } +// init registers the redis driver func init() { cache.Register("redis", &driver{}) } From 11c0c3cb3c45b97fe40289f1f59344dd4e737320 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Sat, 28 Sep 2013 09:09:03 -0400 Subject: [PATCH 8/9] Cleanup Redis keys after tests and benchmarks --- cache/redis/redis_bench_test.go | 47 +++++++++++++++++++++++++++++++-- cache/redis/tx_test.go | 41 ++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/cache/redis/redis_bench_test.go b/cache/redis/redis_bench_test.go index 59cfe00..74aa054 100644 --- a/cache/redis/redis_bench_test.go +++ b/cache/redis/redis_bench_test.go @@ -2,7 +2,6 @@ // Use of this source code is governed by the BSD 2-Clause license, // which can be found in the LICENSE file. -// Benchmarks two different redis schemeas package redis import ( @@ -29,6 +28,10 @@ func BenchmarkSuccessfulFindUser(b *testing.B) { b.Error("found user mismatch", *foundUser, testUser) } } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveUser(testUser)) + b.StartTimer() } func BenchmarkFailedFindUser(b *testing.B) { @@ -66,6 +69,10 @@ func BenchmarkSuccessfulFindTorrent(b *testing.B) { b.Error("found torrent mismatch", foundTorrent, testTorrent) } } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveTorrent(testTorrent)) + b.StartTimer() } func BenchmarkFailFindTorrent(b *testing.B) { @@ -97,6 +104,10 @@ func BenchmarkSuccessfulClientWhitelisted(b *testing.B) { b.Error("peerID not found", testPeerID) } } + // Cleanup + b.StopTimer() + panicOnErr(tx.UnWhitelistClient(testPeerID)) + b.StartTimer() } func BenchmarkFailClientWhitelisted(b *testing.B) { @@ -126,6 +137,11 @@ func BenchmarkRecordSnatch(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { panicOnErr(tx.RecordSnatch(testUser, testTorrent)) } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveTorrent(testTorrent)) + panicOnErr(tx.RemoveUser(testUser)) + b.StartTimer() } func BenchmarkMarkActive(b *testing.B) { @@ -139,6 +155,10 @@ func BenchmarkMarkActive(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { panicOnErr(tx.MarkActive(testTorrent)) } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveTorrent(testTorrent)) + b.StartTimer() } func BenchmarkAddSeeder(b *testing.B) { @@ -155,6 +175,10 @@ func BenchmarkAddSeeder(b *testing.B) { panicOnErr(tx.AddSeeder(testTorrent, testSeeder)) } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveTorrent(testTorrent)) + b.StartTimer() } func BenchmarkRemoveSeeder(b *testing.B) { @@ -172,6 +196,10 @@ func BenchmarkRemoveSeeder(b *testing.B) { panicOnErr(tx.RemoveSeeder(testTorrent, testSeeder)) } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveTorrent(testTorrent)) + b.StartTimer() } func BenchmarkSetSeeder(b *testing.B) { @@ -191,6 +219,10 @@ func BenchmarkSetSeeder(b *testing.B) { tx.SetSeeder(testTorrent, testSeeder) } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveTorrent(testTorrent)) + b.StartTimer() } func BenchmarkIncrementSlots(b *testing.B) { @@ -203,6 +235,10 @@ func BenchmarkIncrementSlots(b *testing.B) { for bCount := 0; bCount < b.N; bCount++ { panicOnErr(tx.IncrementSlots(testUser)) } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveUser(testUser)) + b.StartTimer() } func BenchmarkLeecherFinished(b *testing.B) { @@ -221,6 +257,10 @@ func BenchmarkLeecherFinished(b *testing.B) { panicOnErr(tx.LeecherFinished(testTorrent, testLeecher)) } + // Cleanup + b.StopTimer() + panicOnErr(tx.RemoveTorrent(testTorrent)) + b.StartTimer() } // This is a comparision to the Leecher finished function @@ -241,5 +281,8 @@ func BenchmarkRemoveLeecherAddSeeder(b *testing.B) { panicOnErr(tx.RemoveLeecher(testTorrent, testLeecher)) panicOnErr(tx.AddSeeder(testTorrent, testLeecher)) } - + // Cleanup + b.StopTimer() + tx.RemoveTorrent(testTorrent) + b.StartTimer() } diff --git a/cache/redis/tx_test.go b/cache/redis/tx_test.go index 4d39b47..3d9772c 100644 --- a/cache/redis/tx_test.go +++ b/cache/redis/tx_test.go @@ -43,6 +43,8 @@ func TestFindUserSuccess(t *testing.T) { if *foundUser != *testUser { t.Error("found user mismatch", *foundUser, testUser) } + // Cleanup + panicOnErr(tx.RemoveUser(testUser)) } func TestFindUserFail(t *testing.T) { @@ -83,6 +85,8 @@ func TestFindTorrentSuccess(t *testing.T) { if !reflect.DeepEqual(foundTorrent, testTorrent) { t.Error("found torrent mismatch", foundTorrent, testTorrent) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestFindTorrentFail(t *testing.T) { @@ -107,6 +111,8 @@ func TestRemoveTorrent(t *testing.T) { if found { t.Error("removed torrent found", foundTorrent) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestClientWhitelistSuccess(t *testing.T) { @@ -119,6 +125,8 @@ func TestClientWhitelistSuccess(t *testing.T) { if !found { t.Error("peerID not found", testPeerID) } + // Cleanup + panicOnErr(tx.UnWhitelistClient(testPeerID)) } func TestClientWhitelistFail(t *testing.T) { @@ -161,6 +169,9 @@ func TestRecordSnatch(t *testing.T) { if foundTorrent.Snatches != torrentSnatches+1 { t.Error("snatch not recorded to cached torrent") } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) + panicOnErr(tx.RemoveUser(testUser)) } func TestMarkActive(t *testing.T) { @@ -179,6 +190,8 @@ func TestMarkActive(t *testing.T) { if testTorrent.Active != true { t.Error("cached torrent not activated") } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestClientWhitelistRemove(t *testing.T) { @@ -211,6 +224,8 @@ func TestAddSeeder(t *testing.T) { if found && foundSeeder != *testSeeder { t.Error("seeder not added to local", testSeeder) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestAddLeecher(t *testing.T) { @@ -230,6 +245,8 @@ func TestAddLeecher(t *testing.T) { if found && foundLeecher != *testLeecher { t.Error("leecher not added to local", testLeecher) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestRemoveSeeder(t *testing.T) { @@ -251,6 +268,8 @@ func TestRemoveSeeder(t *testing.T) { if found || foundSeeder == *testSeeder { t.Error("seeder not removed from cache", foundSeeder, *testSeeder) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestRemoveLeecher(t *testing.T) { @@ -271,6 +290,8 @@ func TestRemoveLeecher(t *testing.T) { if found || foundLeecher == *testLeecher { t.Error("leecher not removed from local", foundLeecher, *testLeecher) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestSetSeeder(t *testing.T) { @@ -295,6 +316,8 @@ func TestSetSeeder(t *testing.T) { if foundSeeder != *testSeeder { t.Error("seeder not updated in local", foundSeeder, *testSeeder) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestSetLeecher(t *testing.T) { @@ -318,6 +341,8 @@ func TestSetLeecher(t *testing.T) { if foundLeecher != *testLeecher { t.Error("leecher not updated in local", testLeecher) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestIncrementSlots(t *testing.T) { @@ -336,6 +361,8 @@ func TestIncrementSlots(t *testing.T) { if testUser.Slots != numSlots+1 { t.Error("local slots not incremented") } + // Cleanup + panicOnErr(tx.RemoveUser(testUser)) } func TestDecrementSlots(t *testing.T) { @@ -354,6 +381,8 @@ func TestDecrementSlots(t *testing.T) { if testUser.Slots != numSlots-1 { t.Error("local slots not incremented") } + // Cleanup + panicOnErr(tx.RemoveUser(testUser)) } func TestLeecherFinished(t *testing.T) { @@ -384,6 +413,8 @@ func TestLeecherFinished(t *testing.T) { if foundSeeder == *testLeecher { t.Error("leecher not removed from local", testLeecher) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } // Add, update, verify remove @@ -408,6 +439,8 @@ func TestUpdatePeer(t *testing.T) { if seeder, exists := testTorrent.Seeders[models.PeerMapKey(testSeeder)]; exists { t.Error("seeder not removed from local", seeder) } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestParallelFindUser(t *testing.T) { @@ -435,6 +468,8 @@ func TestParallelFindUser(t *testing.T) { t.Error("found user mismatch", *foundUser, testUserSuccess) } } + // Cleanup + panicOnErr(tx.RemoveUser(testUserSuccess)) } func TestParallelFindTorrent(t *testing.T) { @@ -462,6 +497,8 @@ func TestParallelFindTorrent(t *testing.T) { t.Error("torrent found", foundTorrent) } } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrentSuccess)) } func TestParallelSetSeeder(t *testing.T) { @@ -492,6 +529,8 @@ func TestParallelSetSeeder(t *testing.T) { t.Error("seeder not updated in local", foundSeeder, *testSeeder) } } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } func TestParallelAddLeecher(t *testing.T) { @@ -519,4 +558,6 @@ func TestParallelAddLeecher(t *testing.T) { t.Error("leecher not added to local", testLeecher) } } + // Cleanup + panicOnErr(tx.RemoveTorrent(testTorrent)) } From f113bfb84671d62430277db72f8c89c8f05f5fa4 Mon Sep 17 00:00:00 2001 From: cpb8010 Date: Sat, 28 Sep 2013 11:03:21 -0400 Subject: [PATCH 9/9] Use value order independent response parsing --- cache/redis/redis.go | 211 ++++++++++++++++++++++--------------------- 1 file changed, 108 insertions(+), 103 deletions(-) diff --git a/cache/redis/redis.go b/cache/redis/redis.go index 64eb885..3b70069 100644 --- a/cache/redis/redis.go +++ b/cache/redis/redis.go @@ -112,85 +112,92 @@ func (tx *Tx) close() { tx.Conn.Close() } -// createUser takes a slice of length 7 and returns a pointer to a new models.User or an error. -// This function is used to create a user from a Redis hash response. -// The 7 strings in the slice must be in the specified order. -// -// User.ID -// User.Passkey -// User.UpMultiplier -// User.DownMultiplier -// User.Slots -// User.SlotsUsed -// User.Snatches -// -// If the strings cannot be converted to the correct type, +// createUser takes a string slice of length 14 and returns a pointer to a new +// models.User or an error. +// This function is used to create a user from a Redis hash response(HGETALL). +// The order of strings the in the slice must follow the pattern: +// [, , , , ...] +// If the field value string cannot be converted to the correct type, // createUser will return a nil user and the conversion error. func createUser(userVals []string) (*models.User, error) { - if len(userVals) != 7 { + if len(userVals) != 14 { return nil, ErrCreateUser } var user models.User - convErrors := make([]error, 7) - user.ID, convErrors[0] = strconv.ParseUint(userVals[0], 10, 64) - user.Passkey = userVals[1] - user.UpMultiplier, convErrors[2] = strconv.ParseFloat(userVals[2], 64) - user.DownMultiplier, convErrors[3] = strconv.ParseFloat(userVals[3], 64) - user.Slots, convErrors[4] = strconv.ParseInt(userVals[4], 10, 64) - user.SlotsUsed, convErrors[5] = strconv.ParseInt(userVals[5], 10, 64) - user.Snatches, convErrors[6] = strconv.ParseUint(userVals[6], 10, 64) - - for i := 0; i < 7; i++ { - if convErrors[i] != nil { - return nil, convErrors[i] + var err error + for index, userString := range userVals { + switch userString { + case "id": + user.ID, err = strconv.ParseUint(userVals[index+1], 10, 64) + case "passkey": + user.Passkey = userVals[index+1] + case "up_multiplier": + user.UpMultiplier, err = strconv.ParseFloat(userVals[index+1], 64) + case "down_multiplier": + user.DownMultiplier, err = strconv.ParseFloat(userVals[index+1], 64) + case "slots": + user.Slots, err = strconv.ParseInt(userVals[index+1], 10, 64) + case "slots_used": + user.SlotsUsed, err = strconv.ParseInt(userVals[index+1], 10, 64) + case "snatches": + user.Snatches, err = strconv.ParseUint(userVals[index+1], 10, 64) + } + if err != nil { + return nil, err } } return &user, nil } -// createTorrent takes a slice of length 7 and returns a pointer to a new models.Torrent or an error. -// This function can be used to create a torrent from a Redis hash response. -// The 7 hash fields must be in the specified order. -// -// torrent.ID -// torrent.Infohash -// torrent.Active -// torrent.Snatches -// torrent.UpMultiplier -// torrent.DownMultiplier -// torrent.LastAction -// +// createTorrent takes a string slice of length 14 and returns a pointer to a new models.Torrent +// or an error. +// This function can be used to create a torrent from a Redis hash response(HGETALL). +// The order of strings the in the slice must follow the pattern: +// [, , , , ...] // This function calls multiple redis commands, it's not internally atomic. -// After converting the torrent's ID, the seeders and leechers are populated by getPeers -// If the strings cannot be converted to the correct type, +// If the field values cannot be converted to the correct type, // createTorrent will return a nil user and the conversion error. +// After converting the torrent fields, the seeders and leechers are populated by redis.getPeers func (tx *Tx) createTorrent(torrentVals []string) (*models.Torrent, error) { - if len(torrentVals) != 7 { + if len(torrentVals) != 14 { return nil, ErrCreateTorrent } var torrent models.Torrent - convErrors := make([]error, 9) - torrent.ID, convErrors[0] = strconv.ParseUint(torrentVals[0], 10, 64) - torrent.Infohash = torrentVals[1] - torrent.Active, convErrors[2] = strconv.ParseBool(torrentVals[2]) - torrent.Snatches, convErrors[3] = strconv.ParseUint(torrentVals[3], 10, 32) - torrent.UpMultiplier, convErrors[4] = strconv.ParseFloat(torrentVals[4], 64) - torrent.DownMultiplier, convErrors[5] = strconv.ParseFloat(torrentVals[5], 64) - torrent.LastAction, convErrors[6] = strconv.ParseInt(torrentVals[6], 10, 64) - torrent.Seeders, convErrors[7] = tx.getPeers(torrent.ID, SeedersPrefix) - torrent.Leechers, convErrors[8] = tx.getPeers(torrent.ID, LeechersPrefix) - - for i := 0; i < 9; i++ { - if convErrors[i] != nil { - return nil, convErrors[i] + var err error + for index, torrentString := range torrentVals { + switch torrentString { + case "id": + torrent.ID, err = strconv.ParseUint(torrentVals[index+1], 10, 64) + case "infohash": + torrent.Infohash = torrentVals[index+1] + case "active": + torrent.Active, err = strconv.ParseBool(torrentVals[index+1]) + case "snatches": + torrent.Snatches, err = strconv.ParseUint(torrentVals[index+1], 10, 32) + case "up_multiplier": + torrent.UpMultiplier, err = strconv.ParseFloat(torrentVals[index+1], 64) + case "down_multiplier": + torrent.DownMultiplier, err = strconv.ParseFloat(torrentVals[index+1], 64) + case "last_action": + torrent.LastAction, err = strconv.ParseInt(torrentVals[index+1], 10, 64) } + if err != nil { + return nil, err + } + } + torrent.Seeders, err = tx.getPeers(torrent.ID, SeedersPrefix) + if err != nil { + return nil, err + } + torrent.Leechers, err = tx.getPeers(torrent.ID, LeechersPrefix) + if err != nil { + return nil, err } return &torrent, nil } -// setPeer writes or overwrites peer information. -// The hash fields are sent in a specific order -// so that they can be unpacked correctly +// setPeer writes or overwrites peer information, stored as a Redis hash. +// The hash fields names are the same as the JSON tags on the models.Peer struct. func (tx *Tx) setPeer(peer *models.Peer) error { hashKey := tx.conf.Prefix + getPeerHashKey(peer) _, err := tx.Do("HMSET", hashKey, @@ -274,47 +281,47 @@ func (tx *Tx) addPeers(peers map[string]models.Peer, peerTypePrefix string) erro } // createPeer takes a slice of length 9 and returns a pointer to a new models.Peer or an error. -// This function is used to create a peer from a Redis hash response. -// The 9 strings in the slice must be in the specified order. -// -// peer.ID -// peer.UserID -// peer.TorrentID -// peer.IP -// peer.Port -// peer.Uploaded -// peer.Downloaded -// peer.Left -// peer.LastAnnounce -// -// If the strings cannot be converted to the correct type, -// This function will return a nil peer and the conversion error. +// This function is used to create a peer from a Redis hash response(HGETALL). +// The order of strings the in the slice must follow the pattern: +// [, , , , ...] +// If the field value string cannot be converted to the correct type, +// the function will return a nil peer and the conversion error. func createPeer(peerVals []string) (*models.Peer, error) { - if len(peerVals) != 9 { + if len(peerVals) != 18 { return nil, ErrCreatePeer } var peer models.Peer - convErrors := make([]error, 9) - peer.ID = peerVals[0] - peer.UserID, convErrors[1] = strconv.ParseUint(peerVals[1], 10, 64) - peer.TorrentID, convErrors[2] = strconv.ParseUint(peerVals[2], 10, 64) - peer.IP = peerVals[3] - peer.Port, convErrors[4] = strconv.ParseUint(peerVals[4], 10, 64) - peer.Uploaded, convErrors[5] = strconv.ParseUint(peerVals[5], 10, 64) - peer.Downloaded, convErrors[6] = strconv.ParseUint(peerVals[6], 10, 64) - peer.Left, convErrors[7] = strconv.ParseUint(peerVals[7], 10, 64) - peer.LastAnnounce, convErrors[8] = strconv.ParseInt(peerVals[8], 10, 64) - - for i := 0; i < 9; i++ { - if convErrors[i] != nil { - return nil, convErrors[i] + var err error + for index, peerString := range peerVals { + switch peerString { + case "id": + peer.ID = peerVals[index+1] + case "user_id": + peer.UserID, err = strconv.ParseUint(peerVals[index+1], 10, 64) + case "torrent_id": + peer.TorrentID, err = strconv.ParseUint(peerVals[index+1], 10, 64) + case "ip": + peer.IP = peerVals[index+1] + case "port": + peer.Port, err = strconv.ParseUint(peerVals[index+1], 10, 64) + case "uploaded": + peer.Uploaded, err = strconv.ParseUint(peerVals[index+1], 10, 64) + case "downloaded": + peer.Downloaded, err = strconv.ParseUint(peerVals[index+1], 10, 64) + case "left": + peer.Left, err = strconv.ParseUint(peerVals[index+1], 10, 64) + case "last_announce": + peer.LastAnnounce, err = strconv.ParseInt(peerVals[index+1], 10, 64) + } + if err != nil { + return nil, err } } return &peer, nil } // getPeers returns a map of peers from a specified torrent's peer set(seeders or leechers). -// This is a multiple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic. func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[string]models.Peer, err error) { peers = make(map[string]models.Peer) setKey := tx.conf.Prefix + peerTypePrefix + strconv.FormatUint(torrentID, 36) @@ -325,7 +332,7 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin // Keys map to peer objects stored in hashes for _, peerHashKey := range peerStrings { hashKey := tx.conf.Prefix + peerHashKey - peerVals, err := redis.Strings(tx.Do("HVALS", hashKey)) + peerVals, err := redis.Strings(tx.Do("HGETALL", hashKey)) if err != nil { return nil, err } @@ -342,9 +349,8 @@ func (tx *Tx) getPeers(torrentID uint64, peerTypePrefix string) (peers map[strin } // AddTorrent writes/overwrites torrent information and saves peers from both peer sets. -// The hash fields are sent in a specific order -// so that they can be unpacked correctly -// This is a multiple action command, it's not internally atomic +// The hash fields names are the same as the JSON tags on the models.Torrent struct. +// This is a multiple action command, it's not internally atomic. func (tx *Tx) AddTorrent(t *models.Torrent) error { hashkey := tx.conf.Prefix + TorrentPrefix + t.Infohash _, err := tx.Do("HMSET", hashkey, @@ -392,8 +398,7 @@ func (tx *Tx) RemoveTorrent(t *models.Torrent) error { } // AddUser writes/overwrites user information to a Redis hash. -// The hash fields are sent in a specific order -// so that they can be unpacked correctly +// The hash fields names are the same as the JSON tags on the models.user struct. func (tx *Tx) AddUser(u *models.User) error { hashkey := tx.conf.Prefix + UserPrefix + u.Passkey _, err := tx.Do("HMSET", hashkey, @@ -421,13 +426,13 @@ func (tx *Tx) RemoveUser(u *models.User) error { return nil } -// FindUser returns true and a pointer to a new user struct, if the user exists +// FindUser returns a pointer to a new user struct and true if the user exists, // or nil and false if the user doesn't exist. // This function does not return an error if the torrent doesn't exist. func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { hashkey := tx.conf.Prefix + UserPrefix + passkey // Consider using HGETALL instead of HVALS here for robustness - userStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) + userStrings, err := redis.Strings(tx.Do("HGETALL", hashkey)) if err != nil { return nil, false, err } else if len(userStrings) == 0 { @@ -440,12 +445,12 @@ func (tx *Tx) FindUser(passkey string) (*models.User, bool, error) { return foundUser, true, nil } -// FindTorrent returns a pointer to a new torrent struct and true, if the torrent exists +// FindTorrent returns a pointer to a new torrent struct and true if the torrent exists, // or nil and false if the torrent doesn't exist. -// This is a multiple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic. func (tx *Tx) FindTorrent(infohash string) (*models.Torrent, bool, error) { hashkey := tx.conf.Prefix + TorrentPrefix + infohash - torrentStrings, err := redis.Strings(tx.Do("HVALS", hashkey)) + torrentStrings, err := redis.Strings(tx.Do("HGETALL", hashkey)) if err != nil { return nil, false, err } else if len(torrentStrings) == 0 { @@ -485,7 +490,7 @@ func (tx *Tx) UnWhitelistClient(peerID string) error { // RecordSnatch increments the snatch counter on the torrent and user by one. // This modifies the arguments as well as the hash field in Redis. -// This is a multiple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic. func (tx *Tx) RecordSnatch(user *models.User, torrent *models.Torrent) error { torrentKey := tx.conf.Prefix + TorrentPrefix + torrent.Infohash @@ -546,7 +551,7 @@ func (tx *Tx) MarkInactive(torrent *models.Torrent) error { // AddLeecher adds a new peer to a torrent's leecher set. // This modifies the torrent argument, as well as the torrent's set and peer's hash in Redis. // This function does not return an error if the leecher already exists. -// This is a multiple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic. func (tx *Tx) AddLeecher(torrent *models.Torrent, peer *models.Peer) error { setKey := tx.conf.Prefix + LeechersPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer)) @@ -611,7 +616,7 @@ func (tx *Tx) LeecherFinished(torrent *models.Torrent, peer *models.Peer) error // AddSeeder adds a new peer to a torrent's seeder set. // This modifies the torrent argument, as well as the torrent's set and peer's hash in Redis. // This function does not return an error if the seeder already exists. -// This is a multiple action command, it's not internally atomic +// This is a multiple action command, it's not internally atomic. func (tx *Tx) AddSeeder(torrent *models.Torrent, peer *models.Peer) error { setKey := tx.conf.Prefix + SeedersPrefix + strconv.FormatUint(torrent.ID, 36) _, err := tx.Do("SADD", setKey, getPeerHashKey(peer))