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

feat: Implement running multiple servers #58

Merged
merged 1 commit into from
Oct 23, 2022

Conversation

hhkim0729
Copy link
Collaborator

작업 내용

리뷰어에게

  • Master의 멤버 변수 workers를 포인터형으로 선언하니까 무한히 worker가 추가되는 문제가 해결되었습니다. (부수적으로 temporarily down 문제도 해결) 아직 정확한 동작 원리는 모릅니다.
  • 클라이언트 하나가 최초에 연결될 때 소켓을 2개 생성하는데, 이유는 찾지 못했습니다. (다중 서버 이전에도 동일하게 작동하고 있었음)
  • 간헐적으로 클라이언트가 요청을 보내면 실제로 존재하는 path임에도 콘솔에 not found 에러가 출력되는 경우가 있습니다. 그러나 클라이언트는 요청을 보내지 않았던 것처럼 보이는 상태이며, 다시 요청을 보내면 잘 동작합니다. 이 문제가 언제 발생하는지 정확한 패턴을 알 수가 없어 고치지 못했습니다.

- Create Nginxs class which contains masters and config objects
- Manage all the I/O operations with one poll() in Nginxs class

Additional Tasks
- Move .conf file set to conf directory

Co-authored-by: heehkim <[email protected]>
@hhkim0729 hhkim0729 added the feature New feature or request label Oct 22, 2022
@hhkim0729 hhkim0729 requested a review from srngch October 22, 2022 10:08
@hhkim0729 hhkim0729 merged commit c6b310b into develop Oct 23, 2022
@hhkim0729 hhkim0729 deleted the feature/57/multiple-server branch October 23, 2022 05:04
@srngch srngch mentioned this pull request Oct 23, 2022
Copy link
Owner

@srngch srngch left a comment

Choose a reason for hiding this comment

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

늦었지만 코드 다 읽었습니다 😵
해결 방식이랑 남은 해결해야하는 부분은 모여서 이야기해요!

std::vector<Worker *>::iterator itBegin = workers_.begin();
std::vector<Worker *>::iterator itEnd = workers_.end();

while (itBegin != itEnd) {
Copy link
Owner

Choose a reason for hiding this comment

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

이 부분에서 leak 문제 없을까요? for문으로 사용할 수 없을지 검색해보다가, 포인터 배열 다루다가 메모리 누수 많이 난다고 봐서요..!

Copy link
Collaborator Author

@hhkim0729 hhkim0729 Oct 23, 2022

Choose a reason for hiding this comment

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

앗 찾았습니다... for문으로 바꾸는 게 깔끔할 것 같아요~! 저거 작성할 때는 저희가 이터레이터 쓰는 방법을 잘 모를 때 저렇게 작성했던 것 같습니다. 이전에 Master에 있던 코드 그대로입니다..

누수는 추후에 리팩토링 하면서 다시 확인해봐야 할 것 같습니다.

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

Successfully merging this pull request may close these issues.

Execute multiple server
3 participants