ManageIQ / pg-pglogical

A ruby gem for configuring and using pglogical
Apache License 2.0
3 stars 11 forks source link

Show all subscriptions defined on global region, even if no corresponding node exists on remote #20

Closed yrudman closed 6 years ago

yrudman commented 6 years ago

https://bugzilla.redhat.com/show_bug.cgi?id=1540688

Before: We were showing only properly set-up subscriptions. If one of remote node was not set-up properly than whole UI screen was not loaded. before

After: Show all subscriptions defined on global region, even if no corresponding node exists on remote

after

Issue could be replicated by removing one subscription on global region and immediately adding it back.

After merging this PR corrupted subscription would be shown without any indication that it is invalid. I will work on follow-up PR to show subscription status.

@miq-bot add-label bug, replication, gaprindashvili/yes

\cc @carbonin

miq-bot commented 6 years ago

@yrudman Cannot apply the following labels because they are not recognized: replication, gaprindashvili/yes

carbonin commented 6 years ago

@yrudman can you try to write up a test for this?

It would be in the integration test area we have for this gem.

Maybe if we remove the row from pg_replication_origin_status and ensure that the subscription status method still returns the values we expect before we delete the subscription here

Also, what does the subscription status look like when the origin is not present? Does it show as down? If not, we may want to drop in a status for this type of situation in this patch. What do you think?

yrudman commented 6 years ago

@carbonin status shown as down

yrudman commented 6 years ago

@carbonin I think we should change status to something else since down is misleading. Is corrupted sounds appropriate ?

miq-bot commented 6 years ago

Checked commits https://github.com/yrudman/pg-pglogical/compare/fcf2fe4b21ecb2778b80c40e879d4ad3017644b8~...e5c9fc92f53ea099001919f6c4d311ef9500667b with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 2 files checked, 0 offenses detected Everything looks fine. :trophy:

carbonin commented 6 years ago

So it looks like we're not going to be able to test this after all.

If a subscription is active, we can't remove the replication origin record so we can't really simulate this error case.

This is what we get when we try:

vmdb_production=# SELECT * FROM pg_replication_origin_drop('pgl_vmdb_production_region_0_region_062d953e');
ERROR:  could not drop replication origin with OID 1, in use by PID 2382
giofontana commented 6 years ago

Team, I opened a BZ yesterday for something similar (https://bugzilla.redhat.com/show_bug.cgi?id=1546902). However the screenshot I have is a little bit different (see below). Will this PR fix this issue also?

yrudman commented 6 years ago

@giofontana it looks like your error shown as 'plain' HTML instead of nice dialog box.

Could you scroll down of your error page and check if there is the same text present: "... for nil:NilClass [ops/pglogical_subscription_form_field] ".

giofontana commented 6 years ago

Hi @yrudman , thanks for your reply. I found the "pglogical_subscriptions_form_fields" text. Attached is the entire html.

html_error.txt

yrudman commented 6 years ago

@giofontana actual error is slightly different than original one, but this PR should fix it too.