Тестирование однократного состояния в Golang

Question

У меня есть модуль, который основывается на заполнении кэша вызовом внешнего сервиса, например:

func (provider *Cache) GetItem(productId string, skuId string, itemType string) (*Item, error) {

    // First, create the key we'll use to uniquely identify the item
    key := fmt.Sprintf("%s:%s", productId, skuId)

    // Now, attempt to get the concurrency control associated with the item key
    // If we couldn't find it then create one and add it to the map
    var once *sync.Once
    if entry, ok := provider.lockMap.Load(key); ok {
        once = entry.(*sync.Once)
    } else {
        once = &sync.Once{}
        provider.lockMap.Store(key, once)
    }

    // Now, use the concurrency control to attempt to request the item
    // but only once. Channel any errors that occur
    cErr := make(chan error, 1)
    once.Do(func() {

        // We didn't find the item in the cache so we'll have to get it from the partner-center
        item, err := provider.client.GetItem(productId, skuId)
        if err != nil {
            cErr <- err
            return
        }

        // Add the item to the cache
        provider.cache.Store(key, &item)
    })

    // Attempt to read an error from the channel; if we get one then return it
    // Otherwise, pull the item out of the cache. We have to use the select here because this is
    // the only way to attempt to read from a channel without it blocking
    var sku interface{}
    select {
    case err, ok := <-cErr:
        if ok {
            return nil, err
        }
    default:
        item, _ = provider.cache.Load(key)
    }

    // Now, pull out a reference to the item and return it
    return item.(*Item), nil
}

Этот метод работает так, как я ожидаю. Моя проблема - тестирование; специально проверяя, чтобы убедиться, что метод GetItem вызывается только один раз для данного значения ключа. Мой тестовый код приведен ниже:

var _ = Describe("Item Tests", func() {

    It("GetItem - Not cached, two concurrent requests - Client called once", func() {

        // setup cache

        // Setup a wait group so we can ensure both processes finish
        var wg sync.WaitGroup
        wg.Add(2)

        // Fire off two concurrent requests for the same SKU
        go runRequest(&wg, cache)
        go runRequest(&wg, cache)
        wg.Wait()

        // Check the cache; it should have one value
        _, ok := cache.cache.Load("PID:SKUID")
        Expect(ok).Should(BeTrue())

        // The client should have only been requested once
        Expect(client.RequestCount).Should(Equal(1)) // FAILS HERE
    })
})

// Used for testing concurrency
func runRequest(wg *sync.WaitGroup, cache *SkuCache) {
    defer wg.Done()
    sku, err := cache.GetItem("PID", "SKUID", "fakeitem")
    Expect(err).ShouldNot(HaveOccurred())
}

type mockClient struct {
    RequestFails    bool
    RequestCount    int
    lock            sync.Mutex
}

func NewMockClient(requestFails bool) *mockClient {
    return &mockClient{
        RequestFails:    requestFails,
        RequestCount:    0,
        lock:            sync.Mutex{},
    }
}

func (client *mockClient) GetItem(productId string, skuId string) (item Item, err error) {
    defer GinkgoRecover()

    // If we want to simulate client failure then return an error here
    if client.RequestFails {
        err = fmt.Errorf("GetItem failed")
        return
    }

    // Sleep for 100ms so we can more accurately simulate the request latency
    time.Sleep(100 * time.Millisecond)

    // Update the request count
    client.lock.Lock()
    client.RequestCount++
    client.lock.Unlock()

    item = Item{
        Id:              skuId,
        ProductId:       productId,
    }

    return
}

Проблема, с которой я столкнулся, заключается в том, что иногда этот тест завершается неудачей, поскольку количество запросов равно 2, когда ожидается, что оно равно 1, в строке с комментариями. Этот сбой не является последовательным, и я не совсем уверен, как отладить эту проблему. Любая помощь будет принята с благодарностью.

Answer 1

Я думаю, что ваши тесты иногда терпят неудачу, потому что ваш кэш не может гарантировать, что он выбирает элементы только один раз, и вам повезло, что тесты поймали это.

Если элемент отсутствует в нем, и 2 одновременных в то же время вызовы подпрограмм Cache.GetItem(), может случиться так, что lockMap.Load() сообщит обоим, что ключ отсутствует на карте, обе подпрограммы создадут sync.Once, и оба сохранят свой экземпляр на карте (очевидно, только один - последний - останется на карте, но ваш кеш этого не проверяет).

Тогда обе программы будут вызывать client.GetItem(), потому что 2 отдельных sync.Once не обеспечивают синхронизацию. Только если используется один и тот же экземпляр sync.Once, только тогда можно гарантировать, что функция, переданная в Once.Do(), будет выполнена только один раз.

Я думаю, что sync.Mutex будет проще и уместнее избежать создания и используя 2 sync.Once здесь.

Или, поскольку вы уже используете sync.Map, вы можете использовать метод Map.LoadOrStore(): создать sync.Once и передать его Map.LoadOrStore(). Если ключ уже находится на карте, используйте возвращенный sync.Once. Если ключ отсутствует на карте, ваш sync.Once будет сохранен в нем, и вы можете использовать его. Это гарантирует, что несколько одновременно работающих подпрограмм не смогут хранить в нем несколько sync.once экземпляров.

Примерно так:

var once *sync.Once
if entry, loaded := provider.lockMap.LoadOrStore(key, once); loaded {
    // Was already in the map, use the loaded Once
    once = entry.(*sync.Once)
}

Это решение все еще не идеально: если 2 подпрограммы вызывают Cache.GetItem() в то же время только один попытается извлечь элемент из клиента, но если это не удастся, только эта процедура сообщит об ошибке, другая программа не будет пытаться получить элемент из клиента, но загрузит его из карту, и вы не проверяете, удалась ли загрузка. Вы должны, и если его нет на карте, это означает, что другая, одновременная попытка не смогла получить его. И поэтому вы должны сообщить об ошибке (и очистить sync.Once).

Как видите, все становится сложнее. Я придерживаюсь своего предыдущего совета: здесь было бы проще использовать sync.Mutex.