Skip to content

Commit

Permalink
[TT-12417] Do not delete keys on synchronization (#6642)
Browse files Browse the repository at this point in the history
<details open>
<summary><a href="https://tyktech.atlassian.net/browse/TT-12417"
title="TT-12417" target="_blank">TT-12417</a></summary>
  <br />
  <table>
    <tr>
      <th>Summary</th>
<td>Api keys are lost in worker gateways when keyspace sync
interrupted</td>
    </tr>
    <tr>
      <th>Type</th>
      <td>
<img alt="Bug"
src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium"
/>
        Bug
      </td>
    </tr>
    <tr>
      <th>Status</th>
      <td>In Dev</td>
    </tr>
    <tr>
      <th>Points</th>
      <td>N/A</td>
    </tr>
    <tr>
      <th>Labels</th>
<td><a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20customer_bug%20ORDER%20BY%20created%20DESC"
title="customer_bug">customer_bug</a>, <a
href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20jira_escalated%20ORDER%20BY%20created%20DESC"
title="jira_escalated">jira_escalated</a></td>
    </tr>
  </table>
</details>
<!--
  do not remove this marker as it will break jira-lint's functionality.
  added_by_jira_lint
-->

---

<!-- Provide a general summary of your changes in the Title above -->

Avoiding key deletion when synchronizing. This will avoid having
inconsistent key data between master and slave Redis.
<!-- Describe your changes in detail -->

https://tyktech.atlassian.net/browse/TT-12417?atlOrigin=eyJpIjoiYWNiZTdlNmYwODY5NDI1ZDkzYmQ1MWFlZjM5NGQ3ZTgiLCJwIjoiaiJ9
<!-- This project only accepts pull requests related to open issues. -->
<!-- If suggesting a new feature or change, please discuss it in an
issue first. -->
<!-- If fixing a bug, there should be an issue describing it with steps
to reproduce. -->
<!-- OSS: Please link to the issue here. Tyk: please create/link the
JIRA ticket. -->

https://tyktech.atlassian.net/browse/TT-12417?atlOrigin=eyJpIjoiYWNiZTdlNmYwODY5NDI1ZDkzYmQ1MWFlZjM5NGQ3ZTgiLCJwIjoiaiJ9
<!-- Why is this change required? What problem does it solve? -->

<!-- Please describe in detail how you tested your changes -->
<!-- Include details of your testing environment, and the tests -->
<!-- you ran to see how your change affects other areas of the code,
etc. -->
<!-- This information is helpful for reviewers and QA. -->

<!-- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Refactoring or add test (improvements in base code or adds test
coverage to functionality)

<!-- Go over all the following points, and put an `x` in all the boxes
that apply -->
<!-- If there are no documentation updates required, mark the item as
checked. -->
<!-- Raise up any additional concerns not covered by the checklist. -->

- [ ] I ensured that the documentation is up to date
- [ ] I explained why this PR updates go.mod in detail with reasoning
why it's required
- [ ] I would like a code coverage CI quality gate exception and have
explained why

(cherry picked from commit cea1df4)
  • Loading branch information
mativm02 authored and Tyk Bot committed Oct 24, 2024
1 parent 5bb00d3 commit 148c328
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
2 changes: 2 additions & 0 deletions gateway/rpc_storage_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,8 @@ func (r *RPCStorageHandler) CheckForReload(orgId string) bool {
r.CheckForReload(orgId)
}
} else if !strings.Contains(err.Error(), "Cannot obtain response during") {
forcer := rpc.NewSyncForcer(r.Gw.StorageConnectionHandler, r.buildNodeInfo)
forcer.SetFirstConnection(true)
log.Warning("[RPC STORE] RPC Reload Checker encountered unexpected error: ", err)
}

Expand Down
6 changes: 6 additions & 0 deletions gateway/rpc_storage_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,15 @@ import (
"net/http"
"testing"

<<<<<<< HEAD

Check failure on line 11 in gateway/rpc_storage_handler_test.go

View workflow job for this annotation

GitHub Actions / Go 1.22.x Redis 7

missing import path

Check failure on line 11 in gateway/rpc_storage_handler_test.go

View workflow job for this annotation

GitHub Actions / godoc / Detect Go API changes

missing import path
"github.com/TykTechnologies/tyk/rpc"

"github.com/TykTechnologies/tyk/apidef"
=======

Check failure on line 15 in gateway/rpc_storage_handler_test.go

View workflow job for this annotation

GitHub Actions / Go 1.22.x Redis 7

missing import path
"github.com/TykTechnologies/tyk/internal/model"
"github.com/TykTechnologies/tyk/rpc"

>>>>>>> cea1df441... [TT-12417] Do not delete keys on synchronization (#6642)

Check failure on line 19 in gateway/rpc_storage_handler_test.go

View workflow job for this annotation

GitHub Actions / Go 1.22.x Redis 7

missing import path

Check failure on line 19 in gateway/rpc_storage_handler_test.go

View workflow job for this annotation

GitHub Actions / Go 1.22.x Redis 7

illegal character U+0023 '#'
"github.com/TykTechnologies/tyk/config"

"github.com/lonelycode/osin"
Expand Down
41 changes: 33 additions & 8 deletions rpc/synchronization_forcer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,55 @@ package rpc

import (
"errors"
"sync"

"github.com/TykTechnologies/tyk/apidef"
"github.com/TykTechnologies/tyk/storage"
)

type SyncronizerForcer struct {
store *storage.RedisCluster
getNodeDataFunc func() []byte
store *storage.RedisCluster
getNodeDataFunc func() []byte
isFirstConnection bool
}

var (
syncForcerInstance *SyncronizerForcer
syncForcerOnce sync.Once
)

// NewSyncForcer returns a new syncforcer with a connected redis with a key prefix synchronizer-group- for group synchronization control.
func NewSyncForcer(controller *storage.ConnectionHandler, getNodeDataFunc func() []byte) *SyncronizerForcer {
sf := &SyncronizerForcer{}
sf.getNodeDataFunc = getNodeDataFunc
sf.store = &storage.RedisCluster{KeyPrefix: "synchronizer-group-", ConnectionHandler: controller}
sf.store.Connect()
syncForcerOnce.Do(func() {
sf := &SyncronizerForcer{}
sf.store = &storage.RedisCluster{KeyPrefix: "synchronizer-group-", ConnectionHandler: controller}
sf.store.Connect()
sf.getNodeDataFunc = getNodeDataFunc
sf.isFirstConnection = true

syncForcerInstance = sf
})

if syncForcerInstance != nil {
syncForcerInstance.getNodeDataFunc = getNodeDataFunc
}

return syncForcerInstance
}

func (sf *SyncronizerForcer) SetFirstConnection(isFirstConnection bool) {
sf.isFirstConnection = isFirstConnection
}

return sf
func (sf *SyncronizerForcer) GetIsFirstConnection() bool {
return sf.isFirstConnection
}

// GroupLoginCallback checks if the groupID key exists in the storage to turn on/off ForceSync param.
// If the the key doesn't exists in the storage, it creates it and set ForceSync to true
func (sf *SyncronizerForcer) GroupLoginCallback(userKey string, groupID string) interface{} {
shouldForce := false
shouldForce := sf.isFirstConnection
sf.SetFirstConnection(false)

_, err := sf.store.GetKey(groupID)
if err != nil && errors.Is(err, storage.ErrKeyNotFound) {
Expand Down

0 comments on commit 148c328

Please sign in to comment.