Skip to content
This repository has been archived by the owner on Oct 3, 2021. It is now read-only.

Feedback loop between multiple servers with push enabled #50

Closed
MrPetovan opened this issue May 7, 2018 · 21 comments
Closed

Feedback loop between multiple servers with push enabled #50

MrPetovan opened this issue May 7, 2018 · 21 comments
Labels

Comments

@MrPetovan
Copy link
Collaborator

When we receive a URL through the submit endpoint, we immediately store this URL in the sync-push-queue table. We then submit this URL to the sync-targets during the cron_sync task. However, if the sync-targets have push enabled as well, the same profile keep being submitted between the two or more directories.

First of all, a missing ordering key in the sync-push-queue made that the same profiles always ended at the top of the list, which prevented the rest of the list from being processed as they were re-added almost instantly, receiving them from the remote directory(ies?).

Second of all, there should be a submit cooldown for profiles. If a profile has been submitted in the last X hours, there's no point in processing it again, which would effectively break the feedback loop, as it wouldn't be added to the push queue, etc...

Third of all, this was causing massive back up queue in MySQL while trying to simultaneously delete tags during the submit procedure and retrieving tags for search. We need a better way of updating the tags, especially if they didn't change between two submissions.

@MrPetovan MrPetovan added the bug label May 7, 2018
@MrPetovan
Copy link
Collaborator Author

An initial workaround has been to add a new id auto increment column with a PRIMARY index on it in the sync-push-queue. This requires to change the existing PRIMARY index to UNIQUE before adding the column.

This effectively transforms the table into an actual queue where new items are added to the end of the queue.

The INSERT INTO `sync-push-queue` also has to be changed to REPLACE to avoid duplicate index errors.

@MrPetovan
Copy link
Collaborator Author

I also commented out the DELETE FROM `tag` query to avoid congestion.

@AndyHee
Copy link
Contributor

AndyHee commented May 7, 2018

So do you recommend a specific setting for the time being to avoid the issue.

Everyone only pulls, but doesn't push?

dir.friendi.ca | 0 | 1 | 1523116804
dir.friendica.social | 1 | 1 | 1525699804
socialdir.isurf.ca | 1 | 1 | 1525697404

@AndyHee
Copy link
Contributor

AndyHee commented May 7, 2018

An initial workaround has been to add a new id auto increment column with a PRIMARY index on it in the sync-push-queue. This requires to change the existing PRIMARY index to UNIQUE before adding the column.

Could you commit those changes to your branch, so we can see in more detail what's changed in the DB?

@MrPetovan
Copy link
Collaborator Author

I will tonight EST.

Push is interesting for faster contact discovery, but it currently is unbridled. I would recommend to turn it off until you have changed the behavior.

@AndyHee
Copy link
Contributor

AndyHee commented May 7, 2018

Done.

@AndyHee
Copy link
Contributor

AndyHee commented May 8, 2018

Wow!!

Converting the tag table to InnoDB made an astonishing difference in terms of performance. We probably should covert the entire DB.

I learned a lot about using phpMyAdmin. It's great!

@MrPetovan
Copy link
Collaborator Author

Did you measure increased performance before changing fields?

@AndyHee
Copy link
Contributor

AndyHee commented May 8, 2018

I didn't exactly measure it, but previously maybe it took about three/ four seconds just to load the directory's main page. It was extremely slow, even from my local network where the server is. Now in a "split second" everything is displayed.

@AndyHee
Copy link
Contributor

AndyHee commented May 8, 2018

Yes and the size of the table was reduced to 8.5 MiB from 517MiB.

@MrPetovan
Copy link
Collaborator Author

The size drop is explained by the enforcement of a unique key (747000 rows before, 3800 after), the switch from profile nurl to profile id, and dropping the useless id column.

I’m glad it’s faster just by changing the data structure!

@AndyHee
Copy link
Contributor

AndyHee commented May 8, 2018

I've noticed this error in the log.

2018/05/08 23:42:20 [error] 24826#24826: *127814 FastCGI sent in stderr: "PHP message: PHP Deprecated:  Non-static method Net_Ping::factory() should not be called statically in /var/www/directory/include/site-health.php on line 113
PHP message: PHP Deprecated:  Non-static method Net_Ping::_setSystemName() should not be called statically in /var/www/directory/vendor/pear/net_ping/Net/Ping.php on line 141
PHP message: PHP Deprecated:  Non-static method Net_Ping::_setPingPath() should not be called statically in /var/www/directory/vendor/pear/net_ping/Net/Ping.php on line 143
PHP message: PHP Deprecated:  Non-static method Net_Ping_Result::factory() should not be called statically in /var/www/directory/vendor/pear/net_ping/Net/Ping.php on line 404" while reading response header from upstream, client: 163.172.11.188, server: dir.hubup.pro, request: "GET /submit?url=68747470733a2f2f667269656e646963612e78797a2f70726f66696c652f77616e6465726e HTTP/1.1", upstream: "fastcgi://unix:/var/run/php/php7.1-fpm.sock:", host: "dir.hubup.pro"

@MrPetovan
Copy link
Collaborator Author

Which PHP version?

@AndyHee
Copy link
Contributor

AndyHee commented May 9, 2018

PHP Version 7.1.16-1

@MrPetovan
Copy link
Collaborator Author

Ugh, this is due to Net_Ping having been written in the previous century.

Either we maintain an updated version of the lib, or we make the calling code uglier.

@AndyHee
Copy link
Contributor

AndyHee commented May 9, 2018

Ahh.. Net_Ping is unmaintained. The last release was written for PHP 4.0.0 almost nine years ago!

I take, it there is no existing alternative, ready for use. How much work would it involve to bring the lib up-to-date?

@MrPetovan
Copy link
Collaborator Author

There has been recent alternatives, but none allow to specify the number of packet sent.

It wouldn't be too much work just to suppress this warning. But I feel like it would be a long-term decision, with more surprises along the way. A local workaround might be more appropriate.

@MrPetovan
Copy link
Collaborator Author

MrPetovan commented Nov 5, 2018

I ended up forking pear/Net_ping and I'm using it in the next version of the directory: https://github.com/MrPetovan/Net_Ping

There still is an undefined index issue when the remote server rejects the ping, so it will require a little bit more work.
Also this: https://github.com/pear/Net_Ping/blob/d1f370b3a6072e5a493ed855804d98aadf818fa7/Net/Ping.php#L982-L987

@annando
Copy link
Collaborator

annando commented Nov 5, 2018

Is that some test code?

@MrPetovan
Copy link
Collaborator Author

Looks like some debug code that was committed in the Pear package source.

@MrPetovan
Copy link
Collaborator Author

Won't fix.

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

No branches or pull requests

3 participants