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: multiple concurrent sendFileMessage issues #804

Closed
wants to merge 1 commit into from

Conversation

liamcho
Copy link
Contributor

@liamcho liamcho commented Oct 20, 2023

This has two issues tied to it. So both issues must be resolved before we can merge this.

현재 React UIKit MFM QA 중 발견된 이슈들에 대한 공유 드릴게요. :

  1. 여러파일 동시 선택 전송시하는 경우중에 이미지 1 개, 이미지가 아닌 파일들 1개 이상으로 보내게 되면, 저희가 정한 스펙대로라면 이미지 파일이 먼저 전송이 되어야 합니다. 하지만 현재 React에서는 이미지가 뒤에 보내집니다. 이 원인은 image compression 로직이 오래 걸리는데 asynchronous하게 처리되고 있기 때문입니다. 그래서 이 이슈를 해결하려면 image compression 이 끝난 후에 sendFileMessages() 를 호출하게끔 수정을 해야합니다.
  2. 여러 non-image파일 동시 선택 전송시 (6개 이상), 앱이 멈춘다 (feeze/unresponsive). 이 원인은 sendFileMessage() 의 onPending() 이 짧은 시간 (100ms 언더) 안에 여러번 호출되면서 뷰에 message component 들을 여러번 동시에 업데이트 하게되면서 과부하가 발생하기 때문입니다. 이 문제를 해결하기 위해서는 React UIKit 에 특정 time interval 에 모아둔 pendingMessage 들을 한번에 묶어서 뷰에 업데이트 해주게끔 수정을 해야합니다. 그럴러면 아래처럼 GlobalQueue class를 새로 추가해줘야 합니다.
class GlobalPendingMessageQueue {
  private queue = [];
  private currentTimeout = null;
  private flush = null;
  constructor({ addPendingMessagesToView }) {
    this.flush = addPendingMessagesToView;
  }

  function push(pendingMessage) {
    this.queue.push(pendingMessage);
    if (!this.currentTimeout) {
      this.currentTimeout = setTimeout(() => {
        this.flush(queue);
        this.queue = [];
      }, 100);
    }
  }
}

위 이슈들에 대한 저와
@HoonBaek 의 생각:

  • 1번은 minor, 2번은 major/critical bug 로 생각됩니다.
  • 둘 다 story point 2 이상이 필요합니다.
  • 두 이슈 모두 MFM feature 과는 직접적인 관계가 없습니다.

그래서 저희가 결정할수 있는 선택 옵션이 두가지가 있습니다:

  1. 둘 다 KTLO로 가져간다 (월요일 릴리즈 미포함). 이후 2번을 우선수위로 빠르게 처리한다.
  2. 1번은 KTLO로 가져가고 2번은 React 만 이번 릴리즈에만 임시로 여러파일 동시 선택 전송시에 메세지를 하나씩 보내게끔 동작을 바꾼다 (동시 전송이 아닌 하나씩 전송; 이 동작은 현재 머지된 상태라 월요일 릴리즈 일정까지 문제 없음). 이후에 KTLO 로 위에 2번에 제시된 GlobalQueue 를 적용해서 픽스를 내보낸다.

이번 릴리즈에는 2번 솔루선이 선택되었습니다.

What we settle for the next release:
Reelia-recording-2023-10-20 (2)

@HoonBaek @AhyoungRyu @bang9

@liamcho liamcho requested a review from HoonBaek October 20, 2023 06:06
@liamcho liamcho self-assigned this Oct 20, 2023
@HoonBaek HoonBaek added the DO_NOT_MERGE Do not merge this PR yet label Oct 20, 2023
@liamcho liamcho changed the title bug-fix: Put back multiple sendFileMessage logic to original fix: multiple concurrent sendFileMessage issues May 27, 2024
@liamcho
Copy link
Contributor Author

liamcho commented May 27, 2024

closing this PR as issue will be dealt separately in the future.

@liamcho liamcho closed this May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO_NOT_MERGE Do not merge this PR yet
Projects
None yet
2 participants