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

Adjust data sync order when message event triggered #1659

Closed
windmemory opened this issue Nov 26, 2018 · 4 comments
Closed

Adjust data sync order when message event triggered #1659

windmemory opened this issue Nov 26, 2018 · 4 comments

Comments

@windmemory
Copy link
Member

Recently I was tweaking the wechaty-puppet-padpro project, and found a sync issue related to message sync order.

Here is the code

public async ready (): Promise<void> {
    log.verbose('Message', 'ready()')

    if (this.isReady()) {
      return
    }

    this.payload = await this.puppet.messagePayload(this.id)

    if (!this.payload) {
      throw new Error('no payload')
    }

    const fromId = this.payload.fromId
    const roomId = this.payload.roomId
    const toId   = this.payload.toId

    if (fromId) {
      await this.wechaty.Contact.load(fromId).ready()
    }
    if (roomId) {
      await this.wechaty.Room.load(roomId).ready()
    }
    if (toId) {
      await this.wechaty.Contact.load(toId).ready()
    }
  }

Here is my case:

I logged in an account which has about 6k unread messages from several rooms, then I found thousands of getContactPayload call made to the weixin server which I think is scary. So I looked into the logic of how we load data, I found this piece of code above.

In this piece of code, when we receive a new message, we will sync all related data to it. But I found that when we sync data for the room and room members (two api calls), we will load the data for the toContact and the fromContact, then we don't need to sync it separately in underlying layer.

Today I encountered that there are 4k messages in one room, and sync up 20 messages one time. With the current logic, it will fire up 20 getContact call to weixin server before it loads the room data. (Since the messages are fired up with event, so all these sync operations are parallel executed, thus 20 api call will be triggered together). I think it might be risky to do this, so I suggest to adjust the order of syncing data here to be room -> fromContact -> toContact

@huan
Copy link
Member

huan commented Nov 26, 2018

The PR is ok for me.

However, I did not understand the case that you described: We should already have an underlying cache layer to cache the XXXPayload so that we only to load it once?

If so, why did the order matter to us?

@windmemory
Copy link
Member Author

The case is when we don't have the XXXPayload loaded and we receive a lot of messages requires that payload, like we logged in for the first time. And we receive a lot of messages before we synced all the data.

@huan
Copy link
Member

huan commented Nov 26, 2018

Ok, thanks for the explanation.

It still seems no differences to me, but the changes has merged from PR and it had been published.

@windmemory
Copy link
Member Author

Thanks for merge :)

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

No branches or pull requests

2 participants