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

archive invalid events and drop existing invalid postgres triggers #6181

Conversation

tirumaraiselvan
Copy link
Contributor

This is the backport of #6179 to master

@tirumaraiselvan tirumaraiselvan changed the title archive invalid events and drop existing event triggers archive invalid events and drop existing invalid postgres triggers Nov 12, 2020
@codingkarthik
Copy link
Contributor

@tirumaraiselvan LGTM, but is there any way to test this?

@tirumaraiselvan
Copy link
Contributor Author

tirumaraiselvan commented Nov 12, 2020

This is the way I tested this:

  1. create table test(id serial primary key, name text);
  2. Create 2 event triggers:
{ "type": "replace_metadata",
  "args":
{
  "version": 2,
  "tables": [
    {
      "table": {
        "schema": "public",
        "name": "test"
      },
      "event_triggers": [
        {
          "name": "test_trigger",
          "definition": {
            "enable_manual": false,
            "insert": {
              "columns": "*"
            }
          },
          "retry_conf": {
            "num_retries": 0,
            "interval_sec": 10,
            "timeout_sec": 60
          },
          "webhook": "http://httpbin.org/post"
        },
        {
          "name": "test_trigger2",
          "definition": {
            "enable_manual": false,
            "insert": {
              "columns": "*"
            }
          },
          "retry_conf": {
            "num_retries": 0,
            "interval_sec": 10,
            "timeout_sec": 60
          },
          "webhook": "http://httpbin.org/post"
        }
      ]
    }
  ]
}
}
  1. Drop one of the event triggers:
{ "type": "replace_metadata",
  "args":
{
  "version": 2,
  "tables": [
    {
      "table": {
        "schema": "public",
        "name": "test"
      },
      "event_triggers":[
        {
          "name": "test_trigger2",
          "definition": {
            "enable_manual": false,
            "insert": {
              "columns": "*"
            }
          },
          "retry_conf": {
            "num_retries": 0,
            "interval_sec": 10,
            "timeout_sec": 60
          },
          "webhook": "http://httpbin.org/post"
        }
      ]
    }
  ]
}
}
  1. insert into test, 2 events will be created (one for valid event trigger, one for deleted event trigger): "table or event-trigger not found in schema cache" error in logs #5461
  2. Upgrade with the fix in this PR
  3. Notice that the invalid event is now archived

@@ -24,3 +24,30 @@ $$;
CREATE TRIGGER event_trigger_table_name_update_trigger
AFTER UPDATE ON hdb_catalog.hdb_table
FOR EACH ROW EXECUTE PROCEDURE hdb_catalog.event_trigger_table_name_update();


-- this clears pre-existing invalid triggers
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
-- this clears pre-existing invalid triggers
-- this clears pre-existing invalid triggers
-- See https://github.com/hasura/graphql-engine/pull/6137

Copy link
Collaborator

@jberryman jberryman left a comment

Choose a reason for hiding this comment

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

Just that comment for helpful context

hasura-bot added a commit that referenced this pull request Nov 16, 2020
GITHUB_PR_NUMBER: 6181
GITHUB_PR_URL: #6181

Co-authored-by: Tirumarai Selvan A <tiru@hasura.io>
GitOrigin-RevId: fdbc18a
@hasura-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants