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

Deregistration process manager retry the rollup errors from aggregates #1473

Merged

Conversation

CDimonaco
Copy link
Member

Description

Deregistration process manager needs to retry the rolling up errors from aggregates.

The other errors will be captured by the default implementation, so the process manager process will stop.

@CDimonaco CDimonaco added the elixir Pull requests that update Elixir code label May 30, 2023
@CDimonaco CDimonaco self-assigned this May 30, 2023
Copy link
Contributor

@arbulu89 arbulu89 left a comment

Choose a reason for hiding this comment

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

Nice @CDimonaco

Checking this reminded me that we might want to change the stream_roll_up_event_handler.ex and include the "deregistration` events there as well

@@ -289,6 +289,17 @@ defmodule Trento.DeregistrationProcessManager do
}
end

# Retry the rollup errors, stop the process on other errors

def error({:error, :host_rolling_up}, _command_or_event, %{context: context}),
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want something more generic:

@error_states = [:host_rolling_up, :cluster_rolling_up, :sap_system_rolling_up]
...
def error({:error, error}, _command_or_event, %{context: context}), do
    when error in @error_states do, {:retry, context}

@fabriziosestito
Copy link
Member

Nice @CDimonaco

Checking this reminded me that we might want to change the stream_roll_up_event_handler.ex and include the "deregistration` events there as well

@arbulu89 I don't think deregistration should cause a rollup, I think those events should be skipped. A roll-up will be triggered by the tombstone event if any.

@CDimonaco CDimonaco merged commit e9ffacb into deregistration May 30, 2023
@CDimonaco CDimonaco deleted the deregistration_process_manager_error_handling branch May 30, 2023 12:12
@CDimonaco
Copy link
Member Author

Nice @CDimonaco
Checking this reminded me that we might want to change the stream_roll_up_event_handler.ex and include the "deregistration` events there as well

@arbulu89 I don't think deregistration should cause a rollup, I think those events should be skipped. A roll-up will be triggered by the tombstone event if any.

Yep, this is in the other pr #1425, currently is in draft because is blocked by #1461, but you can see the logic done for clusters and hosts, only sap system are blocked

@arbulu89
Copy link
Contributor

Nice @CDimonaco
Checking this reminded me that we might want to change the stream_roll_up_event_handler.ex and include the "deregistration` events there as well

@arbulu89 I don't think deregistration should cause a rollup, I think those events should be skipped. A roll-up will be triggered by the tombstone event if any.

Actually, i was thinking more if getting those events could create some pattern matching error, as we don't have them in the list. But the events handlers ignore not matched handles, so we are fine

arbulu89 added a commit that referenced this pull request Jun 29, 2023
* Host deregistration events (#1245)

* Host deregistration commands

* Host deregistration events

* Fixing module attributes in host deregistration commands

* Fix deregistration commands namespace

* Deregistration side effects (#1252)

* Add deregistered_at to read model

Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com>
Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>

* Handle side effects from HostDeregistered event

Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com>
Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>

* Filter deregistered hosts from get_all_hosts

Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com>
Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>

* Fix tests

Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com>
Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Add test for filtering unregistered hosts

Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com>
Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Host Deregistration process manager and aggregate handling (#1249)

* Host aggregate handling of deregistration event

* Wip Deregistration process manager

* Process Managers supervisor

* Add Deregistration process manager test

* Fix credo and mispells

* Squashed Process manager state into process manager

* Routing deregistration commands to Host aggregate

* Fix deregistration process manager state with deftype

* Format router file

* Deregistration timestamp carried from Deregistration requested command

* Fix spell in process manager docstring

* Fix typo in test description

* Cluster and cluster host deregistration events/commands (#1274)

* Cluster and cluster host deregistration events/commands

* Address comments

* Remove leading space

* Fix Hosts query without the soft deleted hosts exclusion (#1282)

* Cluster registration also for nodes not DC (#1275)

* Cluster registration process happens also when the discovered node is
not a DC

* Address docs and testing review feedbacks

* Removed useless tests from cluster_tests

* Add host to cluster when a message from a dc host arrives for a cluster

Co-Authored-By: Fabrizio Sestito <fabrizio.sestito@suse.com>

* Add register cluster host test when the host is and is not a DC

---------

Co-authored-by: Fabrizio Sestito <fabrizio.sestito@suse.com>

* Cluster deregistration process manager & aggregate changes (#1278)

* Make Process Manager aware of cluster deregistration

Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Handle cluster host deregistration in the cluster aggregate

Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Route the DeregisterClusterHost to the cluster aggregate


---------

Co-authored-by: Jamie Rodriguez <jamie.rodriguez@suse.com>

* Cluster deregistered side effects (#1280)

* Add deregistered_at field to Cluster read model

* ClusterDeregistered event projection

* Wip on clusters module, get clusters that are not soft deleted

* Clusters entrypoint tests

* Test broadcast of cluster deregistration in cluster projector

* Removed first from request_check_execution query

* Fix query parenthesis in request_clusters_checks_execution

* get_all_clusters, tested for deregistered and soft deleted clusters

* Handle HostRemovedFromCluster in host projection

* Cluster projector reorder of project clauses

* Host projector set the cluster to null when HostRemovedFromCluster

* Correct typos (#1315)

* Database registration changes (#1326)

* Database sap system registration change

The database istance is register only if the sr is disabled or with sr
enabled but only primary instances

* Sap systems moduledoc updated

* Changed error when registering a non primary database for a sap system

* Updated moduledoc of sap system domain

* Refuse database registration changes

The registration is refused if the sap system does not exists
and the database has a secondary role

* Fix sap system domain tests

* Fix typos

* Format

* Add SAP System deregistration commands & events (#1314)



Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Fix Deregistration process manager state validation

* Add application instance deregistration logic (#1357)



Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>

* Application instances registration changes (#1358)

* Add SAP System de/registration logic to Process Manager (#1356)



Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Add database instance deregistration logic (#1372)



Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>
Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Add SAP System deregistration side effects (#1387)


Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Detect ascs/ers cluster type (#1392)

* Detect ascs/ers cluster type

* Update functions order to improve pattern matching

* Ascs ers type frontend (#1398)

* Add ascs/ers cluster type to the frontend

* Improve e2e testing of the clusters view

* Discover ASCS/ERS cluster SIDs (#1400)

* Discover ascs/ers cluster sids

* Add additional_sids to domain and side effects

* Add additional_sids to cluster api schema

* Handle `:database_not_registered` errors (#1405)

Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Add polymorphic embed usage to deftype macro (#1411)

* Properly handle multiple errors returned (#1413)

* Display ASCS/ERS clusters SID in clusters view (#1403)

* Show additional sids in the clusters view

* Adapt e2e tests to check for sids

* Order get_all_clusters output by id

* Revert HeartbeatSucceeded to old name (#1429)

* Add deregistration websocket events (#1424)



Co-authored-by: Jamie Rodríguez <jamie.rodriguez@suse.com>
Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Discover ASCS/ERS clusters health and initial details (#1422)

* Add nodes entry to the crm payload

* Add ascs/ers cluster details value objects

* Implement ascs/ers cluster health discovery

* Update cluster details to the polymorphic embed

* Add tests to the ascs/ers cluster health discovery

* Fix validate_required usage

* Set default value to additional_sids in cluster events (#1441)

* Cluster delta deregistration (#1386)

* Add get_cluster_id_by_host_id/1

* Add delta deregistration logic

* Add additional ascs/ers cluster details discovery (#1442)

* Add additional ascs/ers cluster details discovery

* Add comment to parse_ascs_ers_cluster_nodes

* Wrap long function in do/end

* Set false instead of nil in Enum.filter function

* Deregistration process manager retry the rollup errors from aggregates (#1473)

* Fix HostRemovedFromCluster concurrency issues (#1474)

* Basic deregistration leftovers (#1461)

* test

* Add deregistered_at field to Database

* Fix update of Application and Database when deregistering instances

* Put deregistered_at timestamp instead of nil sid on database deregistration

* Remove sid to nil when Sap system is deregistered

* Emit sap system deregistered when the database is deregistered

* Emit database deregistered event when no instances left and db is not
already deregistered

* Sap systems domain deregistration tests refactored

* format sap system domain

* Emit database deregistered event when they are instances only when the
db is not registered

* Removed redundant nil default to deregistered_at in database

* Fix mispell in sap system tests

* Add fake_sid util to sap system testing suite

* Sap system does not get deregistered a second time

* Changed test titles in sap system test, to remove "a second time"
misleading phrase

* Deregistration tombstoning rollup (#1425)

* Host tombstoning on host deregistration

* Add host tombstone rollup handling in stream event handler

* Cluster tombstoned event and lifespan when cluster is deregistered

* Cluster rollup triggered on Cluster tombstone event

* Move cluster tombstoned event to cluster domain folder

* Sap system aggregate emits tombstone event

* Sap system aggregate tombstoning test

* SapSystem tombstoned event triggers sap system rollup

* Removed tombstoned events from lifespan stop mechanism in all aggregates

* Formatted Cluster/Host tombstoned events

* Fix logging string in stream rollup event handler

* Addressing review feedbacks

* Credo format

* Change redirect plug to use a list of available versions (#1487)

* Change redirect plug to use a list of available versions

* Add sanity check for empty available api versions list

* Use Keyword.get to get the opts

* Enable openapi schema versioning (#1488)

* Convert the open api plug into a versinable macro

* Move V1 schemas to its own folder

* Update controllers to used versioned openapi

* Update router to pipethrough specific open api version

* Disable open api cache on dev

* Update controller tests to use new openapi versions

* Temporarily set openapi V1 version in CI docs generation

* Test paths generation in open api spec

* Add endpoint to deregister a host (#1450)



Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>

* Add V2 scope to /api/clusters endpoint (#1492)

* Add openapi v2 schemas

* Add clusters v2 endpoint controller and view

* Update pheonix router to add v2 endpoints

* Adapt clusters v1 schema and view to be backward compatible

* Use clusters v2 version on projector

* Remove deprecated code

* Use /api/v2/clusters endpoint in frontend

* Improve and fix some new schema implementations

* Aggregate clauses reorder (#1495)

* Reorder host aggregate clauses, host_id: nil clauses compacted

* Sap system clauses sap_system_id: nil, compacted

* Sap system delta deregistration (#1491)

* Fix cluster delta computation

* Add get_application_instances_by_host_id/1 and get_database_instances_by_host_id/1

* Add delta deregistration logic

* Identify sap systems ensa version (#1496)

* Identify sap systems ensa version

* Replace nil by no_ensa atom in EnsaVersion enum

* DatabaseDeregistered side effects (#1504)

* Add deregistered_at field to databases read model

* Handle database deregistered side effects

* Filtered deregistered databases in the usecases

* Make `HostReadModel.last_heartbeat_timestamp` a virtual field (#1505)

* Project removal of db/app instances to the read models (#1507)

* Project removal of db/app instances to the read models

* Reject commands when aggregate deregistered (#1513)

* Host aggregate reject commands when the host is deregistered

* Cluster aggregate reject all commands except registration when
deregistered

* Sap system aggregate rejects commands when deregistered

* Host aggregate rejects updates when deregistered

* Cluster aggregate rejects all commands when deregistered

* Fix host tests

* Ensa version domain (#1512)

* Add ensa_version to SapSystemReadModel

* Add SapSystemUpdated event and domain logic

* Add and update sap system domain tests

* Add SapSystemUpdated projection

* Add new ensa_version field in openapi schema

* Add test to check updated event is not emitted

* Update migration to set default value afterwards

* Create AscsErsClusterRole enum (#1520)

* Host restoration (#1517)

* Host restoration in Host aggregate

* Host restoration projector

* Addressing typo feedbacks

* Move host details update after dergistered_at guard clause

* Broadcast sap system updated (#1516)

* Broadcast SapSystemUpdated event

* Update sap systems in redux state upon update event

* Cluster restoration (#1523)

* Extract cluster update procedure to separate function

* Cluster restoration

* Cluster restored projection

* Addressing review feedbacks

* Addressing review feedbacks

* Move sagas functions to separate files according to adr 0009 (#1531)

* Update health summary endpoint (#1530)

* Update health summary service and view

* Update openapi schema and deprecate old fields

* Polish the code and handle unclustered systems

* Fix function specs

* Remove fixed sid usage from db instance creation in test

* Frontend instance removal on broadcast (#1522)

* Add sid to instance deregistration broadcast

* Remove instances from upon instance deregistration broadcast

* Apply various cleanups to the database state

* Use Repo.get! where possible

* Improve notification message

* Fix wrong test placement

* Add test for instances removal notify

* Fix sap system domain apply ordering issues (#1550)

* Move application instances (#1554)

* Add Deregistration modal (#1537)

* Allow rollup on deregistered aggregates (#1551)

* Complete tests on command acceptance/rejection (#1563)

* Sap system restoration (#1545)

* Database restoration in sap system aggregate

* Database restore projection

* Sap system restore domain

* Sap system restore projection

* Emit application instance registered or moved in restore

* Removed health multi change on sap system restoration

* Remove sap_system_id setting in database restored apply

* Removed sid from DatabaseRestored event

* Removed sid field from SapSytemRestored event

---------

Co-authored-by: Carmine Di Monaco <carmine.dimonaco@suse.com>
Co-authored-by: Rubén Torrero Marijnissen <rtorreromarijnissen@suse.com>
Co-authored-by: Fabrizio Sestito <fabrizio.sestito@suse.com>
Co-authored-by: Jamie Rodriguez <jamie.rodriguez@suse.com>
Co-authored-by: Carmine Di Monaco <carmine.dimonaco@gmail.com>
Co-authored-by: Xabier Arbulu Insausti <xarbulu@suse.com>
Co-authored-by: Eugen Maksymenko <eugen.maksymenko@suse.com>
Co-authored-by: Jurgen Goldschmidt <jurgen.goldschmidt@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
elixir Pull requests that update Elixir code
Development

Successfully merging this pull request may close these issues.

3 participants