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

[FIX] Proxy upload to correct instance #7304

Merged
merged 2 commits into from
Jun 27, 2017

Conversation

rodrigok
Copy link
Member

@RocketChat/core

Proxy the file upload parts to the instance who created the file when running with multiple instances to prevent errors when sticky sessions does not works.
@rodrigok rodrigok added this to the 0.57.0 milestone Jun 21, 2017
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7304 June 21, 2017 21:12 Inactive
@geekgonecrazy
Copy link
Member

geekgonecrazy commented Jun 22, 2017

Curious... Would it be possible to set instance Id when the post happens.
And then proxy the command called here: https://github.com/RocketChat/Rocket.Chat/blob/develop/packages/rocketchat-file-upload/server/lib/FileUpload.js#L282 to the instance that handled the file upload? Instead of proxying the full file upload? It'd be a lot lighter.

I fear what happens if we start proxying full file uploads around multiple instances? Instead of effecting a single instance with an upload we now have two being occupied by the upload.

But... I like this better then uploading to the database and then moving it

@sampaiodiego
Copy link
Member

@geekgonecrazy the problem is with the XHR requests, the way it works now the temp file could be split into multiple instances, so no matter where ufsComplete is called it will not be able to grab all temp files.

@geekgonecrazy
Copy link
Member

@sampaiodiego yeah that's true.. 👍 it works.

Only request I would have is moving the intercepter out of avatars and into regular upload since it's applying to all uploads. Other then that code looks good.

Copy link
Member

@geekgonecrazy geekgonecrazy left a comment

Choose a reason for hiding this comment

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

Tested and changes work as expected

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-7304 June 22, 2017 21:12 Inactive
@rodrigok rodrigok merged commit b031171 into release-candidate Jun 27, 2017
@rodrigok rodrigok deleted the hotfix/rc-avatar-upload-proxy branch June 27, 2017 16:23
@kaji-bikash
Copy link

version: '2'

services:
  rocketchat1:
    image: rocketchat/rocket.chat:0.64.2
    restart: unless-stopped
    volumes:
      - ../shared/uploads:/var/snap/rocketchat-server/common/uploads
    environment:
      - INSTANCE_IP=172.17.43.194
      - PORT=3333
      - ROOT_URL=https://rocket.example.com
      - MONGO_URL=mongodb://mongo1:27017/parties
    ports:
      - 3333:3333

  rocketchat2:
    image: rocketchat/rocket.chat:0.64.2
    restart: unless-stopped
    volumes:
      - ../shared/uploads:/var/snap/rocketchat-server/common/uploads
    environment:
      - INSTANCE_IP=172.17.43.194
      - PORT=4444
      - ROOT_URL=https://rocket.example.com
      - MONGO_URL=mongodb://mongo1:27017/parties
    ports:
      - 4444:4444

  rocketchat3:
    image: rocketchat/rocket.chat:0.64.2
    restart: unless-stopped
    volumes:
      - ../shared/uploads:/var/snap/rocketchat-server/common/uploads
    environment:
      - INSTANCE_IP=172.17.43.194
      - PORT=5555
      - ROOT_URL=https://rocket.example.com
      - MONGO_URL=mongodb://mongo1:27017/parties
    ports:
      - 5555:5555

We have 3000+ users with roughly 300-500 users online. All the rocket.chat instances are running in single server and the spec is 16 GB of RAM with 4 cores inside AWS VPC. We have nginx proxing to the rocketchat ports.

We have frequent problems with rocket.chat

  • not responding timely,
  • message not being sent,
  • frozen response,
  • uploads are not working.
  • .....very slow

We know replica-set would help but anything besides that we could be missing or points that might help our cases.

Much appreciated.

@geekgonecrazy
Copy link
Member

Might try posting that in #7524 so it doesn't get lost

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

Successfully merging this pull request may close these issues.

None yet

5 participants