Skip to content

On Node removing remove related keys from QueryCache (#81) #270

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

Closed

Conversation

SergeiPavlov
Copy link
Contributor

@SergeiPavlov SergeiPavlov commented Aug 11, 2022

This bug appears in following scenareo:

  1. Create some cached query with Node "foo"
  2. call Domain.StorageNodeManager.RemoveNode()
  3. rename schema of the Node
  4. Add node with the same name "foo" but other schema
  5. Invoke the same query which was cached in step 1. - it references the old schema

@alex-kulakov
Copy link
Contributor

alex-kulakov commented Dec 27, 2022

Hello @SergeiPavlov,

Can you reproduce this on current master?

Can you open an issue for this with small [Test] method which shows the problem? This should be applicable to 6.0 and 7.0 too so it should be resolved in 6.0. As I understand this is not a regular case, personally I would expect nodes to be with same settings. If I'm right then I would offer a StorageNodeManager.Remove(string nodeID, bool clearQueryCache = false) method.

@alex-kulakov
Copy link
Contributor

When you offered a united cache for all nodes, I thought about having nodes which would store their portion of QueryCache so instead of one giant cache we would have a number of smaller caches. This would allow to release resources on node removal but in this case all re-added nodes would start working with cold query cache which is drawback.

@SergeiPavlov
Copy link
Contributor Author

Hello @SergeiPavlov,

Can you reproduce this on current master?

Can you open an issue for this with small [Test] method which shows the problem? This should be applicable to 6.0 and 7.0 too so it should be resolved in 6.0. As I understand this is not a regular case, personally I would expect nodes to be with same settings. If I'm right then I would offer a StorageNodeManager.Remove(string nodeID, bool clearQueryCache = false) method.

I hardly have to write DO tests. The case was detected in real app and fixed by this code.

@alex-kulakov
Copy link
Contributor

alex-kulakov commented Dec 28, 2022

Well, I'm pretty sure that I understood the problem, but would be nice to have real example to exclude any misunderstanding, I'll try to reproduce it. How often does such scenario happen? Was I right about rareness of this case?

@alex-kulakov
Copy link
Contributor

I made some changes for this issue for 6.0 branch since it is affected too (PR #296), the changes contain test cases, you can take a look. After merge I'll merge it to 7.0 and then to master with required changes.

@SergeiPavlov
Copy link
Contributor Author

SergeiPavlov commented Dec 28, 2022

Well, I'm pretty sure that I understood the problem, but would be nice to have real example to exclude any misunderstanding, I'll try to reproduce it. How often does such scenario happen? Was I right about rareness of this case?

The scenario is not typical. This is the steps:

  1. We rename schema for tenant, actually we move tenant DB to another server and db
  2. then we update NodeRecord
  3. start getting error like:
SQL error occured.SQL error details 'Type: SyntaxError;'Query 'SELECT ... FROM [oldschema].[User] ...]'Original message 'Invalid object name 'oldschema.User'.'

due to using old schema name

The error was not rare in such a scenario

@alex-kulakov
Copy link
Contributor

Let me put it this way.

  1. How frequently does your app call Domain.Build()? every day, every week, every month?
  2. How frequently does your app add nodes to Domain and remove nodes from Domain? daily? weekly?
  3. How frequently do you move tenants between schemas/DBs/servers/ whatever?

@SergeiPavlov
Copy link
Contributor Author

SergeiPavlov commented Dec 29, 2022

Let me put it this way.

  1. How frequently does your app call Domain.Build()? every day, every week, every month?
  2. How frequently does your app add nodes to Domain and remove nodes from Domain? daily? weekly?
  3. How frequently do you move tenants between schemas/DBs/servers/ whatever?
  1. Hundreeds times per day
  2. In normal mode it only adds thousands times per one Domain
  3. It is manual operation. ~100 times per week

@alex-kulakov
Copy link
Contributor

@SergeiPavlov, the changes from 6.0 with StorageNodeManager.Remove(string nodeId, bool clearQueryCache = false) option were merged to 7.0 and then to master (with required changes for newer code). I can close this PR without merge, right?

@SergeiPavlov
Copy link
Contributor Author

@SergeiPavlov, the changes from 6.0 with StorageNodeManager.Remove(string nodeId, bool clearQueryCache = false) option were merged to 7.0 and then to master (with required changes for newer code). I can close this PR without merge, right?

Yes. I'm cloning it

@SergeiPavlov
Copy link
Contributor Author

Replaced by bb21e6f

@SergeiPavlov SergeiPavlov deleted the upstream/RemoveNode branch January 16, 2023 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants