这是indexloc提供的服务，不要输入任何密码
Skip to content

schema cache syncing without relying on a Postgres table #6173

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
Closed
wants to merge 11 commits into from
Closed

schema cache syncing without relying on a Postgres table #6173

wants to merge 11 commits into from

Conversation

@rakeshkky
Copy link
Member

@rakeshkky rakeshkky commented Nov 11, 2020 •
edited
Loading

Description

This is an incremental PR towards #5797. It avoids using hdb_cataog.hdb_schema_update table and associated trigger to notify schema syncing events to other server instances. Instead it is achieved in a much simpler way by directly using pg_notify procedure to publish event payload.

Other things this PR does:-

  • Now initialiseCtx function is only used in serve command
  • Using new GlobalCtx type for essentials required for all graphql-engine commands

Changelog

  • CHANGELOG.md is updated with user-facing content relevant to this PR. If no changelog is required, then add the no-changelog-required label.

Affected components

  • Server

Limitations, known bugs & workarounds

Server checklist

Catalog upgrade

Does this PR change Hasura Catalog version?

  • No
  • Yes

Metadata

Does this PR add a new Metadata feature?

  • No
  • Yes

Breaking changes

  • No Breaking changes
  • There are breaking changes

@rakeshkky rakeshkky requested a review from 0x777 November 11, 2020 08:35
@rakeshkky rakeshkky self-assigned this Nov 11, 2020
@rakeshkky rakeshkky requested a review from abooij November 11, 2020 08:39
abooij
abooij reviewed Nov 11, 2020
View reviewed changes
Copy link
Contributor

@abooij abooij left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I can't reliably judge the syncing logic itself. It seems to me that the core of tho changes are in fetchLastUpdate, recordSchemaUpdate, and notifySchemaCacheSync. With the old fetchLastUpdate and recordSchemaUpdate, I think I understood the logic: record sends a message to all other graphql-engine instances, and fetch received messages. Is notify the new record or the new fetch? I believe it's the former. If so, where's the new fetch? Where's the code that receives these PG notifications?

Edit: Sorry, perhaps the failing PR was just a flake. [This used to say: @rakeshkky, please invite reviewers after you got the PR to pass CI. Otherwise, you ensure that the reviewer does not review the code that will end up getting merged.]

@rakeshkky
Copy link
Member Author

rakeshkky commented Nov 11, 2020 •
edited
Loading

@0x777 Please take a look at this PR. You have a better context of the schema syncing process than other reviewers.

@rakeshkky rakeshkky requested a review from abooij November 11, 2020 11:07
@netlify
Copy link

netlify bot commented Nov 18, 2020 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit 4c4f9e4

https://deploy-preview-6173--hasura-docs.netlify.app

@rakeshkky rakeshkky requested a review from a team as a code owner November 19, 2020 12:22
hasura-bot added a commit that referenced this pull request Nov 24, 2020
@hasura-bot @rakeshkky @0x777
server: schema cache syncing without relying on a Postgres table (#40)
d955001 
An incremental PR towards # 5797 (OSS repo)

* schema cache syncing without relying on a Postgres table

Co-authored-by: rakeshkky <12475069+rakeshkky@users.noreply.github.com>
Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com>
Co-authored-by: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com>
GITHUB_PR_NUMBER: 6173
GITHUB_PR_URL: #6173

* fix pro compile issues

Co-authored-by: Rakesh Emmadi <12475069+rakeshkky@users.noreply.github.com>
Co-authored-by: Vamshi Surabhi <0x777@users.noreply.github.com>
GitOrigin-RevId: 055b06a
@hasura-bot
Copy link
Contributor

hasura-bot commented Nov 24, 2020

Thanks for your contribution! Your changes have been merged successfully.

@hasura-bot hasura-bot closed this Nov 24, 2020
@sumodgeorge sumodgeorge mentioned this pull request Jan 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@0x777 0x777 Awaiting requested review from 0x777

@abooij abooij Awaiting requested review from abooij

Assignees

@rakeshkky rakeshkky

Labels

c/server Related to server merged no-changelog-required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rakeshkky @hasura-bot @abooij @0x777