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

PHPLIB-1531: Retry once when disabling fail points #1445

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

jmikola
Copy link
Member

@jmikola jmikola commented Sep 24, 2024

https://jira.mongodb.org/browse/PHPLIB-1531

This is an attempt to resolve flaky test failures following #1427

@jmikola jmikola force-pushed the 1.20-phplib-1531-failpoints branch from 2004e98 to 14ec9f0 Compare September 24, 2024 03:28
@jmikola jmikola force-pushed the 1.20-phplib-1531-failpoints branch from 14ec9f0 to fea8b2a Compare September 24, 2024 03:33
@jmikola jmikola marked this pull request as ready for review September 24, 2024 12:43
@jmikola jmikola requested a review from a team as a code owner September 24, 2024 12:43
@jmikola jmikola requested a review from GromNaN September 24, 2024 12:43
$operation = new DatabaseCommand('admin', ['configureFailPoint' => $failPoint, 'mode' => 'off']);
$operation->execute($server);
} catch (ConnectionException $e) {
// Retry once in case the connection was dropped by the last operation
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still at a loss for why this wasn't an issue with the original FailPointObserver. There, the Server object was instantiated from the CommandStartedEvent; however, it's just an integer server_id and Manager reference. That's not much different from the object we're capturing here from configureFailPoint(). In both cases, the command to disable a fail point is still being executed via phongo_execute_command() with the same underlying mongoc_client_t and server_id.

The only difference I can identify is when we're disabling fail points. Previously, FailPointObserver disabled fail points during our equivalent of a tearDown() method. Now, we're doing so immediately after executing operations and before evaluating expectEvents (note: this is what the spec itself suggests). It's possible the previous deferral allowed some opportunity to reestablish a dropped connection.

This may be related to CDRIVER-4532, which highlights an outstanding libmongoc deficiency in the code paths utilized by PHPC.

@jmikola jmikola merged commit 883a2ee into mongodb:v1.20 Sep 24, 2024
36 checks passed
@jmikola jmikola deleted the 1.20-phplib-1531-failpoints branch September 24, 2024 14:37
alcaeus added a commit that referenced this pull request Sep 24, 2024
* Retry once when disabling fail points (#1445)

The previous refactor in 90dcf18 changed when we disable fail points. ManagesFailPointTrait now does so immediately after test operations are run instead of during tear down, so we may be hitting a point where the server stream was disconnected and libmongoc is not recovering (possibly related to CDRIVER-4532). Adding an extra retry attempt seems to overcome this.

* Use non-capturing catch statement

---------

Co-authored-by: Jeremy Mikola <[email protected]>
Co-authored-by: Andreas Braun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants