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

Federation tests replication round1 - demonstrate absent replication of comment deletes #3657

Merged
merged 18 commits into from Jul 27, 2023

Conversation

RocketDerp
Copy link
Contributor

@RocketDerp RocketDerp commented Jul 18, 2023

This is an urgent test addition to highlight the problem with comment deletes not replicating when a remote-server creates the comment. The community home instance has no code to replicate delete of comment to all the downstream subscribed instances. In these tests, Gamma serves as an example of the downstream servers subscribed who are not getting the delete in 0.18.2 version.

The intention here is to put more developer eyes on #3625 as a design issue. I am still working to devise additional tests to see if the problem goes beyond comment deletes.

…n, this is currently failing because gamma does not get the report
@RocketDerp

This comment was marked as abuse.

@RocketDerp

This comment was marked as abuse.

RocketDerp and others added 2 commits July 19, 2023 15:35
* Add http cache for webfingers

* Remove the outgoing cache middleware & adjust the cache headers directive

* Use 1h & 3day cache header

* Update routes and adjust the cache headers location

* revert apub caching

---------

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
Co-authored-by: Felix Ableitner <me@nutomic.com>
// This is testing replication from remote-home-remote (alpha-beta-gamma)
let gammaComment = (
await resolveComment(gamma, commentRes.comment_view.comment)
).comment;
Copy link
Member

@Nutomic Nutomic Jul 19, 2023

Choose a reason for hiding this comment

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

This isnt really testing replication. If the comment failed to be federated, resolve will fetch it instead. You could instead use resolvePost and then getComments for that post to check if its included there. Or add a new getComments variant which retrieves all newest comments for the whole instance.

This comment was marked as abuse.

console.log("gamma has deleted comment", gammaCommentRes.comment.comment);
}
}
expect(gammaCommentRes.error).toBe("couldnt_find_object");
Copy link
Member

Choose a reason for hiding this comment

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

Okay turns out its like I mentioned in my other comment, the problem is in activity_lists.rs. Removing the line PersonInboxActivities::Delete fixes this test. So it enters the wrong variant which doesnt forward it to followers. Have to think about the proper way to fix it, probably some rewrite of that file is necessary.

This comment was marked as abuse.

Copy link
Member

Choose a reason for hiding this comment

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

FYI the way to debug this is by looking at the Lemmy logs in less /tmp/lemmy_beta.out etc, and adding dbg! statements in the Rust code to see what it is doing.


// This is testing replication from remote-home-remote(s) (alpha-beta-gamma)
// The sitaution could be that a remote moderator is on Gamma and needs to see the report in doing their moderator duty.
let gammaReports = await listCommentReports(gamma);
Copy link
Member

Choose a reason for hiding this comment

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

This probably fails because report is not announcable, so it will only go to the community instance but not anywhere else.

This comment was marked as abuse.

@Nutomic
Copy link
Member

Nutomic commented Jul 20, 2023

I pushed a commit to your branch which fixes the federation of deletes. You need to run git pull to get it locally. Please remove the changes to "Report a comment" test so that we can go ahead and merge this. You also need to run prettier on the ts files.

@@ -13,7 +13,7 @@ popd
yarn
yarn api-test || true

killall lemmy_server
killall -s1 lemmy_server
Copy link
Member

Choose a reason for hiding this comment

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

Without this there were still lemmy processes left running after tests completed.

This comment was marked as abuse.

This comment was marked as abuse.

RocketDerp added 2 commits July 20, 2023 06:09
…situation, this is currently failing because gamma does not get the report"

This reverts commit 7bd3b20.
@RocketDerp RocketDerp force-pushed the federation_tests_replication_round1 branch from 8a2bab8 to a72590b Compare July 20, 2023 13:10
@RocketDerp RocketDerp closed this Jul 20, 2023
@RocketDerp RocketDerp reopened this Jul 20, 2023
@RocketDerp

This comment was marked as abuse.

@RocketDerp

This comment was marked as abuse.

@RocketDerp

This comment was marked as abuse.

@RocketDerp RocketDerp closed this Jul 20, 2023
@RocketDerp RocketDerp reopened this Jul 20, 2023
@RocketDerp

This comment was marked as abuse.

@RocketDerp

This comment was marked as abuse.

@Nutomic
Copy link
Member

Nutomic commented Jul 21, 2023

Here is the prettier issue for showing a diff on check. No progress in 4 years...

That tmknom is irrelevant, its just the person who built a dockerfile for prettier which we are using. It shouldnt have any changes compared to other ways of running prettier. I run npx prettier -w {path}, pushed a commit with that to your branch but somehow formatting is still failing. cc @dessalines

@RocketDerp

This comment was marked as abuse.

@Nutomic Nutomic enabled auto-merge (squash) July 27, 2023 09:53
@Nutomic
Copy link
Member

Nutomic commented Jul 27, 2023

There was another failure in api tests which I had to fix.

@Nutomic Nutomic merged commit 21a87eb into LemmyNet:main Jul 27, 2023
1 check passed
Nutomic added a commit that referenced this pull request Jul 27, 2023
…of comment deletes (#3657)

* more robust test of unlike a comment, confirm replication to instance downstream from community home

* more robust 'delete a comment' test, confirm replication

* Far more robust "Report a comment" test. Many comments about situation, this is currently failing because gamma does not get the report

* typo and actually have Gamma comment check use gamma, not alpha

* prepare-drone-federation-test.sh has some more echo output and note about the LEMMY_DATABASE_URL format (#3651)

* Add http cache for webfingers (#3317)

* Add http cache for webfingers

* Remove the outgoing cache middleware & adjust the cache headers directive

* Use 1h & 3day cache header

* Update routes and adjust the cache headers location

* revert apub caching

---------

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
Co-authored-by: Felix Ableitner <me@nutomic.com>

* Rewrite activity lists to fix delete federation (fixes #3625)

* Revert "typo and actually have Gamma comment check use gamma, not alpha"

This reverts commit 7dfb6ee.

* Revert "Far more robust "Report a comment" test. Many comments about situation, this is currently failing because gamma does not get the report"

This reverts commit 7bd3b20.

* prettier TypeScript

* revised comments, as ResolveObject isn't using routine replication

* fmt

* fix api tests

* remove comment

---------

Co-authored-by: cetra3 <cetra3@hotmail.com>
Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
Co-authored-by: Felix Ableitner <me@nutomic.com>
@RocketDerp

This comment was marked as abuse.

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.

None yet

4 participants