Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Sync response for device_lists is not immediate when running with workers #11726

Closed
hyossing opened this issue Jan 12, 2022 · 9 comments
Closed
Assignees
Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Sync defects related to /sync S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@hyossing
Copy link

Description

Hello,

Synapse does not create sync response for device_lists when logout in master and worker mode.

In a single synapse mode, sync response comes for 'device_lists' when logout.

Steps to reproduce

  1. Set Master and Worker environment
    attached zip file is my test environment.
    synapse-worker-deployment.zip

  2. login two devices with same id using element client.

  1. logout from one of the two devices.
  • check sync response from the non-logout device.

Log


Log For Master & Worker

2022-01-12 01:53:24,316 - synapse.replication.tcp.resource - 172 - DEBUG - replication_notifier-46 - Getting stream: device_lists: 19 -> 20

Master sends replication data to worker.

2022-01-12 01:53:24,326 - synapse.replication.tcp.handler - 510 - DEBUG - process-replication-data-86 - Received rdata device_lists (master) -> 20

Worker receives replication data from master. But, nothing happened.


Log For Single Process

  • Single Process
    log_single_process.txt
    For the single process case, sync response is created for device list right after logout.
2022-01-12 02:08:31,602 - synapse.util.metrics - 152 - DEBUG - GET-55 - Entering block _generate_sync_entry_for_device_list


### Version information
- **Version**: 1.49.2
- **Install method**: docker-container
- **Platform**: mac os



@DMRobertson
Copy link
Contributor

Hi @hyossing, can you clarify what you mean by "logout"? I'm guessing you've got element-web opened in two tabs in the same browser, and you close one tab while leaving the other open?

@H-Shay H-Shay added the X-Needs-Info This issue is blocked awaiting information from the reporter label Jan 12, 2022
@hyossing
Copy link
Author

Hello, @DMRobertson

  1. Logout means, click logout menu in element-web client. So, synapse receives POST ..../logout request.
  • If general(single process) case, synapse create sync response for device_lists to notify logout(changed) device.

Screen Shot 2022-01-13 at 7 47 11 AM

  1. No two tabs.
  • It means, two devices. Login to synapse from different browser or physical machine.

And,

I'm sorry about the synapse-worker-deployment.zip.

Some configuration is wrong. It would not work.

  • haproxy.cfg

I updated some configuration.
synapse-worker-deployment_20220113.zip

Thanks you.

@DMRobertson
Copy link
Contributor

DMRobertson commented Jan 17, 2022

Thanks @hyossing. One quick point of clarification, to make sure I understand the problem:

For the single process case, sync response is created for device list right after logout.

Synapse does not create sync response for device_lists when logout in master and worker mode.

In master-worker mode, do you see an entry in device_lists eventually, e.g. in a later incremental sync? Or does it never appear at all?

@hyossing
Copy link
Author

Dear @DMRobertson

It appears eventually in a later incremental sync as you said.

I found one more strange behavior.

The sync response comes right after logout if there is a room.

Screen Shot 2022-01-19 at 7 02 53 AM

But, If there is no room(leave and forget the test room), then the sync response comes later.

Thanks you.

@DMRobertson DMRobertson added X-Needs-Discussion and removed X-Needs-Info This issue is blocked awaiting information from the reporter labels Jan 27, 2022
@DMRobertson
Copy link
Contributor

It appears eventually in a later incremental sync as you said.

This is to be expected when we're running in worker mode. (There are delays as information propagates from one worker to another). We

Is this causing a particular problem for your use case?

@DMRobertson DMRobertson added X-Needs-Info This issue is blocked awaiting information from the reporter A-Sync defects related to /sync A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. and removed X-Needs-Discussion labels Jan 27, 2022
@hyossing
Copy link
Author

hyossing commented Feb 2, 2022

It is a problem for me.

There is a particular behavior of our own client when device list changes.

We expected the client will receive the same event right after logout in worker mode.

  • Before that, we only run single mode.

Currently, we are considering to change our policy for the changed-device-list.

  • Allow delayed device-list change information.

@squahtx squahtx added X-Needs-Discussion and removed X-Needs-Info This issue is blocked awaiting information from the reporter labels Feb 15, 2022
@DMRobertson
Copy link
Contributor

Sorry for the delay in getting back to you. On reflection, I think I may have understood your problem. It sound a bit like #11457: something happens that clients should be told about, but we don't send a /sync response immediately, and have to wait for another event to actually send down the information.

Is the problem reproducible every time, or does it only happen sometimes?

@reivilibre reivilibre changed the title No sync response for device_lists on master and worker mode. Sync response for device_lists is not immediate when running with workers Feb 18, 2022
@hyossing
Copy link
Author

@DMRobertson
Sorry for the delay to reply.

The problem reproducible every time for the no room and worker case as I explained above.

In my opinion, the particular behavior is a trivial issue. We can handle by changing our policy a little bit.

BR
// Hyosung Lim

@erikjohnston erikjohnston added S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Feb 23, 2022
@richvdh
Copy link
Member

richvdh commented May 4, 2022

It sounds like this is expected behaviour and the reporter has found a workaround. Closing.

@richvdh richvdh closed this as completed May 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Sync defects related to /sync S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

6 participants