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.

Sign up for GitHub

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

Jump to bottom

clear events related to schema cache updates (close #2542) #2585

Merged
merged 7 commits into from
Jul 31, 2019
Merged

clear events related to schema cache updates (close #2542) #2585

merged 7 commits into from
Jul 31, 2019

Conversation

rakeshkky
Copy link
Member

Description

Schema syncing on horizontal scaling introduced in #1574 uses a table to record schema cache update events. The table is populated with all events and cleaning never happens. This PR addresses this issue.

Affected components

  • Server

Related Issues

Close #2542

Solution and Design

  • Bump the catalog version with;
    • deleting all rows from hdb_catalog.hdb_schema_update_event table
    • define single row constraint on the table
    • drop existing trigger and create a trigger for each insert and update events on the table
  • Use same instance id for telemetry and schema syncing threads

Notes to Reviewer

Please look for line comments while you're reviewing

Steps to test and verify

Limitations, known bugs & workarounds

@rakeshkky rakeshkky added the c/server Related to server label Jul 22, 2019
@rakeshkky rakeshkky requested a review from 0x777 as a code owner July 22, 2019 12:25
@rakeshkky rakeshkky self-assigned this Jul 22, 2019
@rakeshkky rakeshkky added the s/ok-to-merge Status: This pull request can be merged to master label Jul 22, 2019
@netlify
Copy link

netlify bot commented Jul 22, 2019 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit 4735a57

https://deploy-preview-2585--hasura-docs.netlify.com

rakeshkky
rakeshkky commented Jul 22, 2019
View reviewed changes
server/src-lib/Hasura/Server/Query.hs
INSERT INTO hdb_catalog.hdb_schema_update_event
(instance_id, occurred_at) VALUES ($1::uuid, DEFAULT)
ON CONFLICT ((occurred_at IS NOT NULL))
DO UPDATE SET instance_id = $1::uuid, occurred_at = DEFAULT
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using INSERT instead of UPDATE because after initialise the hdb_catalog.hdb_schema_update_event is empty and insert is required in that case. Using ON CONFLICT clause to update the already present single row.

@hasura-bot
Copy link
Contributor

Review app for commit e93b773 deployed to Heroku: https://hge-ci-pull-2585.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2585-e93b773f

rakeshkky
rakeshkky commented Jul 22, 2019
View reviewed changes
server/stack.yaml.lock
@@ -0,0 +1,103 @@
# This file was autogenerated by Stack.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stack.yaml.lock is introduced in stack build tool from 2.1.1. It is recommended to put it in version control. Refer here to know more.

lexi-lambda
lexi-lambda previously approved these changes Jul 22, 2019
View reviewed changes
Copy link
Contributor

@lexi-lambda lexi-lambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, aside from the two minor comments.

server/src-rsr/initialise.sql Outdated Show resolved Hide resolved
server/src-lib/Hasura/Server/Query.hs Outdated Show resolved Hide resolved
@hasura-bot
Copy link
Contributor

Review app for commit 9da5384 deployed to Heroku: https://hge-ci-pull-2585.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2585-9da53847

lexi-lambda
lexi-lambda previously approved these changes Jul 23, 2019
View reviewed changes
@rakeshkky rakeshkky removed the request for review from 0x777 July 23, 2019 09:23
@hasura-bot
Copy link
Contributor

Review app for commit 4735a57 deployed to Heroku: https://hge-ci-pull-2585.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2585-4735a574

@0x777 0x777 merged commit db24d32 into hasura:master Jul 31, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-2585.herokuapp.com is deleted

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
@rakeshkky @polRk
schema sync process will not generate garbage data (close hasura#2542) (
875d08a 
hasura#2585)

schema syncing logic now requires only a single row in `hdb_schema_update_event`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0x777 0x777 0x777 approved these changes

@lexi-lambda lexi-lambda lexi-lambda left review comments

@shahidhk shahidhk Awaiting requested review from shahidhk

@ecthiender ecthiender Awaiting requested review from ecthiender

Assignees

@rakeshkky rakeshkky

Labels
c/server Related to server s/ok-to-merge Status: This pull request can be merged to master
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

clean up the events related to schema cache updates
4 participants
@rakeshkky @hasura-bot @lexi-lambda @0x777