Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cherry-pick #7869 fix(helpers): fix concurrent map writes #7870

Merged
merged 1 commit into from
Aug 9, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions backend/helpers/pluginhelper/api/ds_remote_api_proxy_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"io"
"net/http"
"strings"
"sync"

"github.com/apache/incubator-devlake/core/errors"
"github.com/apache/incubator-devlake/core/log"
Expand All @@ -32,8 +33,9 @@ import (
// DsRemoteApiProxyHelper is a helper to proxy api request to remote servers
type DsRemoteApiProxyHelper[C plugin.ToolLayerApiConnection] struct {
*ModelApiHelper[C]
logger log.Logger
httpClientCache map[string]*ApiClient
logger log.Logger
httpClientCache map[string]*ApiClient
httpClientCacheMutex *sync.RWMutex
}

// NewDsRemoteApiProxyHelper creates a new DsRemoteApiProxyHelper
Expand All @@ -43,9 +45,10 @@ func NewDsRemoteApiProxyHelper[
modelApiHelper *ModelApiHelper[C],
) *DsRemoteApiProxyHelper[C] {
return &DsRemoteApiProxyHelper[C]{
ModelApiHelper: modelApiHelper,
logger: modelApiHelper.basicRes.GetLogger().Nested("remote_api_helper"),
httpClientCache: make(map[string]*ApiClient),
ModelApiHelper: modelApiHelper,
logger: modelApiHelper.basicRes.GetLogger().Nested("remote_api_helper"),
httpClientCache: make(map[string]*ApiClient),
httpClientCacheMutex: &sync.RWMutex{},
}
}

Expand All @@ -68,9 +71,14 @@ func (rap *DsRemoteApiProxyHelper[C]) getApiClient(connection *C) (*ApiClient, e
key = cacheableConn.GetHash()
}
// try to reuse api client
if key != "" && rap.httpClientCache[key] != nil {
rap.logger.Info("Reused api client")
return rap.httpClientCache[key], nil
if key != "" {
rap.httpClientCacheMutex.RLock()
client, ok := rap.httpClientCache[key]
rap.httpClientCacheMutex.RUnlock()
if ok {
rap.logger.Info("Reused api client")
return client, nil
}
}
// create new client if cache missed
client, err := NewApiClientFromConnection(gocontext.TODO(), rap.basicRes, c.(plugin.ApiConnection))
Expand All @@ -79,7 +87,9 @@ func (rap *DsRemoteApiProxyHelper[C]) getApiClient(connection *C) (*ApiClient, e
}
// cache the client if key is not empty
if key != "" {
rap.httpClientCacheMutex.Lock()
rap.httpClientCache[key] = client
rap.httpClientCacheMutex.Unlock()
} else {
rap.logger.Info("No api client reuse")
}
Expand Down
Loading