Skip to content

Conversation

geoje
Copy link
Member

@geoje geoje commented Sep 13, 2024

냥인🐈🧍‍♀️ 안녕하세요!

이번 미션은 학습할 내용이 대부분이어서 코드보단 PR 내용에 집중하였습니다.

그간 리뷰 덕에 정말 많이 배웠네요 😀
마지막 리뷰도 잘 부탁 드립니다! 🤗

stage 0 FixedThreadPool

테스트 코드 시작 시 아래 newFixedThreadPool 함수를 통해 2개의 스레드를 항상 유지하며 일을 처리하는 스레드풀을 생성합니다.

final var executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(2);

이후 아래 코드를 통해 3번의 logWithSleep() 메서드를 실행합니다.
logWithSleep() 는 실행되면 1초를 기다린 후 입력으로 들어온 문자열을 Logger 로 출력하는 메서드입니다.

executor.submit(logWithSleep("hello fixed thread pools"));
executor.submit(logWithSleep("hello fixed thread pools"));
executor.submit(logWithSleep("hello fixed thread pools"));

학습 테스트의 목적은 위와 같이 3개의 일을 2개의 스레드가 있는 풀에 요청했을 때 executor.getPoolSize() 값과 executor.getQueue().size() 값이 얼마인지 인지하는 것입니다.

executor.getPoolSize() 는 스레드 풀에 현재 존재하는 워커 스레드의 개수를 의미하며 항상 2개를 유지하도록 생성하였으니 2가 될 것입니다.
executor.getQueue().size() 는 대기중인 일 개수를 의미하며 3개의 일 중 2개는 작업 중이므로 나머지 1개가 이 큐에 존재하기에 값은 1이 됩니다.

final int expectedPoolSize = 2;
final int expectedQueueSize = 1;

stage 0 CachedThreadPool

스레드 풀 동작은 위와 동일하지만 newCachedThreadPool() 메서드를 통해 생성할 경우 평소에는 0 개의 스레드가 풀에 있다가 작업이 들어오면 최대 Integer.MAX_VALUE (2^31-1) 개 까지 생성합니다.
또한 keepAliveTime 이 기본으로 60 Sec 이기 때문에 생성된 스레드가 작업이 끝난 후 60 초 동안 다른 작업을 요청이 없으면 제거되는 원리입니다.

따라서 작업 요청 전 executor.getPoolSize()0 이지만 3 개의 작업이 요청되었으므로 최종 값은 아래와 같습니다.

final int expectedPoolSize = 3;
final int expectedQueueSize = 0;

stage 1 synchronized

테스트에서 2개의 작업을 동시에 수행할 때 UserServlet.join() 메서드 첫 줄인 if (!users.contains(user)) 에서 유저 추가 전 검증을 하게 되는데 새로운 유저가 추가되기 전 2개의 스레드 모두가 이 if 문을 통과한다면 유저를 2번 추가하는 동시성 문제 Thread Interference 가 발생하게 됩니다.

따라서 public void 사이에 public synchronized void 를 하게되면 한 스레드가 이 메서드에 접근할 때 이미 다른 스레드가 동작 중이면 끝날 때 까지 대기하게 됩니다.

stage 2 Tomcat thread pool

요청은총 10 번이 이루어지며 이에 따라 서버 설정 및 서버와 클라이언트 로그가 어떻게 찍히는지 알아보았습니다.

Tomcat accept-count: 1 max-connections: 3 threads.min-spare: 2 threads.max: 2

Server Client
[nio-8080-exec-1] http call count : 2
[nio-8080-exec-2] http call count : 1
[nio-8080-exec-1] http call count : 3
[nio-8080-exec-2] http call count : 4
Thread-4 ConnectException
Thread-5 ConnectException
Thread-6 ConnectException
Thread-7 ConnectException
Thread-8 ConnectException
Thread-9 ConnectException
Thread-0 HttpTimeoutException: request timed out
Thread-2 HttpTimeoutException: request timed out
Thread-1 Thread-3 은 정상 응답 받음
Time Stamp Content
200 ms [4개의 클라이언트 요청]
2개는 스레드가 500ms 짜리 작업 실행 threads.max
1개는 서버까지 연결 수락 max-connections - threads.max
1개는 서버 전 OS까지 연결 수락 accept-count
500 ms [추가 6개 클라이언트 요청]
받아줄 공간 없어서 연결 거부 Client-side ConnectException
500 ~ 550 ms 500ms 짜리 작업 완료 후 정상 응답
완료와 동시에 서버까지 연결 수락한 1개와 OS까지 연결 수락한 1개 각각 500ms 짜리 작업 실행
1000 ms Client쪽에서 요청 Timeout 인 1초가 지났으므로 HttpTimeoutException: request timed out 발생
1000 ~ 1050 ms Client가 timeout 되어 응답을 받지 않아도 Server는 작업을 수행했으므로 스레드 nio-8080-exec-1nio-8080-exec-2 에서 로그 출력

@geoje geoje self-assigned this Sep 13, 2024
Copy link
Member

@cutehumanS2 cutehumanS2 left a comment

Choose a reason for hiding this comment

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

새양 ~ PR 내용 보고 감탄했어요. 정성 가득 👍 
무지 상세하고 깔끔합니다.
전에 페어할 때도 새양이 작성하신 PR 내용을 보고 놀랬던 기억이 떠올라요.
한결같아서 좋습니다 ~~

질문과 학습 키워드를 몇 가지 남겨 보았습니다.
읽고 천천히 생각해 보신 뒤, 변경이 필요하다고 느껴지는 부분만 반영해 주세요. ㅎ ㅎ
아마 다음 요청 때 머지할 것 같아요.
즐거운 주말 보내세요. ◠‿◠

Comment on lines +39 to 41
public synchronized void calculate() {
setSum(getSum() + 1);
}
Copy link
Member

Choose a reason for hiding this comment

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

메서드에 동기화를 적용하셨군요. 저도예요. ㅎ ㅎ
이제 calculate() 메서드에는 한 번에 하나의 스레드만 접근할 수 있게 되었습니다.

그런데 만약에 이 메서드에 지금 보다 훨씬 더 많은 스레드가 접근하려고 시도한다면,
왠지 성능 저하가 발생할 것 같아요. 성능 저하를 최소화할 방법이 없을까요?

힌트: 동기화하는 범위를 줄여볼 수 있지 않을까?

Copy link
Member Author

Choose a reason for hiding this comment

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

calculate 메서드에서 범위를 더 줄인다면 setSum 메서드를 걸어볼 수 있을 것 같은데 이렇게 되면 getSum 메서드를 동시에 읽는 문제가 발생하여 결국 데이터 정합성에 어긋날 것 같아요!

LMSthread PDF 14페이지의 이미지를 가져와보았습니다!
image

Copy link
Member Author

Choose a reason for hiding this comment

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

라고 생각했지만 아래 코멘트를 통해 volatile 키워드를 알게 되었고, Main Memory 에서의 데이터를 읽어 데이터 정합성을 지킬 수 있는 것을 알게 되었습니다! 😮

Copy link
Member

Choose a reason for hiding this comment

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

음.. 저도 스레드 및 동기화를 완벽히 마스터하지는 못해서 새양의 코멘트를 보고 지금 긴가민가한 상태인데요!

public void calculate() {
    synchronized(this) {
        setSum(getSum() + 1);
    }
}

이렇게 동기화 블록을 사용하면, 한 스레드가 calculate()를 호출하는 동안 다른 스레드가 setSum()이나 getSum()을 호출하여 sum 값을 수정할 수 있어, 스레드 간섭 문제가 발생하고 데이터의 정합성이 깨진다는 것이죠? 🤔
헷갈리네요... 알려주셔서 감사합니다 새양. ㅎ ㅎ 덕분에 또 배워갑니다. 😸

Comment on lines +19 to +21
* 테스트가 성공하도록 SynchronizedMethods 클래스에 동기화를 적용해보자. synchronized 키워드에 대하여 찾아보고 적용하면 된다.
* <p>
* Guide to the Synchronized Keyword in Java https://www.baeldung.com/java-synchronized
Copy link
Member

Choose a reason for hiding this comment

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

학습 테스트에서 주어진 키워드는 아니지만 다음 키워드들의 차이점도 알고 가면 좋을 것 같아요. 😸

synchronized vs volatile vs AtomicInteger(Atomic 변수)

Copy link
Member Author

Choose a reason for hiding this comment

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

와... Atomic 변수 그동안 동시성 문제를 편하게 해결해 준다고만 생각하고 원리에 대해 공부를 못하였는데 기존 synchornizedvolatile 키워드의 원자성 및 성능 저하 문제를 해결해주는 신기한 친구였군요!

CPU 캐시 메모리와 RAM 메인 메모리의 데이터 정합성 문제를 CAS(Compare And Swap) 으로 해결하였네요!
좋은 학습 이었어요! 감사합니다!!

final var executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(2);
executor.submit(logWithSleep("hello fixed thread pools"));
executor.submit(logWithSleep("hello fixed thread pools"));
executor.submit(logWithSleep("hello fixed thread pools"));
Copy link
Member

Choose a reason for hiding this comment

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

스레드 풀에 작업 처리를 요청하는 방법에는 학습 테스트에서 제시된 submit()외에도 한 가지 방법이 더 있습니다.
이는 무엇이고, 두 방법의 차이점, 그리고 왜 submit()이 더 권장되는지 알고 계신가요? 😸

Copy link
Member Author

@geoje geoje Sep 16, 2024

Choose a reason for hiding this comment

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

https://www.baeldung.com/java-execute-vs-submit-executor-service
여기저기 찾아봐도 제가 알던 느낌과 같아서 일단 해당 게시글을 들고 오게 되었어요!

둘 다 인자로 Runnable 을 받아 스레드에게 Task 를 전달해주는 것은 맞지만 execute 의 리턴 값은 void 인 반면 submit 의 리턴 값은 Future 입니다. 이 반환 값을 통해 작업이 끝났을 때 어떠한 값을 받아 추가 작업을 할 때 유용할 것입니다!

submit 에 2번째 인자로 defaultValue 를 줄 수도 있고 장점은 많지만 리턴 값이 없는 상황에서 굳이 사용해야하나? 라는 느낌을 받긴 했습니다.

모르는 사람이 이 메서드 사용을 볼 때 작업이 끝나고 어떠한 데이터를 받아 추가 작업을 할 것 으로 예상되는데 아무런 행동을 하지 않으면 오히려 이상할 것 같았어요!

Baeldung 의 마지막 부분에 아래 글이 적혀있더라구요!

If we need to obtain the result of a task or handle exceptions, we should use submit(). On the other hand, if we have a task that doesn’t return a result and we want to fire it and forget it, execute() is the right choice.

저도 이 의견에 동의 하는게 어떠한 Task 가 실행하고 그냥 잊어버리면 될 경우 execute 를 사용하는 것이 좋지 않을까요...??

submit 에 권장되는 이유가 궁금합니다!! 😃

Copy link
Member

Choose a reason for hiding this comment

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

오 열심히 학습하셨네요. 새양의 말이 맞습니다.
작업 처리 결과가 필요없다면 execute()를 사용하면 되겠죠!

이미 학습하셔서 알겠지만, execute()는 작업 처리 도중 예외가 발생하면 스레드가 종료되고, 해당 스레드가 스레드 풀에서 제거된다고 합니다. 반면, submit()은 작업 처리 도중 예외가 발생하더라도 스레드가 종료되지 않고 재사용된다고 하네요!
이러한 점에서 가급적이면 "스레드의 생성 오버헤드를 줄이기 위해 submit()을 사용하는 게 좋다"라고 말씀드리고 싶었는데, 괜히 submit()이 권장되는 이유라 해서 조금 혼동을 드렸던 것 같아요. 죄송해요. 😅 맞게 학습하셨습니다!

Comment on lines 14 to 23
private void join(final User user) {
if (!users.contains(user)) {
try {
Thread.sleep(1); // Expected context switching to another thread
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
users.add(user);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

테스트에서 2개의 작업을 동시에 수행할 때 UserServlet.join() 메서드 첫 줄인 if (!users.contains(user)) 에서 유저 추가 전 검증을 하게 되는데 새로운 유저가 추가되기 전 2개의 스레드 모두가 이 if 문을 통과한다면 유저를 2번 추가하는 동시성 문제 Thread Interference 가 발생하게 됩니다.
따라서 public void 사이에 public synchronized void 를 하게되면 한 스레드가 이 메서드에 접근할 때 이미 다른 스레드가 동작 중이면 끝날 때 까지 대기하게 됩니다.

문제 원인에 대해 잘 설명해 주셨습니다.👍
그런데 synchronized 키워드를 추가하지 않아 테스트가 실패합니다.😅

+) 일부러 컨텍스트 스위칭 코드를 추가하신 이유가 궁금해요 ~ 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

앗! 디버거를 사용하지 않고도 빠르게 현상을 확인할 수 있는 방법을 추가하기 위해 넣어두었습니다!

synchoronized 키워드를 추가하여 테스트를 통과하도록 만들어두었습니다!

}
var processor = new Http11Processor(connection);
new Thread(processor).start();
executorService.execute(processor);
Copy link
Member

Choose a reason for hiding this comment

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

submit()과 execute() 중 후자를 선택하신 이유가 궁금해요. 😸

Copy link
Member Author

Choose a reason for hiding this comment

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

ConnectorProcessor 의 작업들을 스레드에 잘 분배해 처리만 해주지 어떠한 결과 값을 받아 후처리를 할 필요가 없다고 판단하였기 때문입니다!

executorService.execute(processor);
}

public void stop() {
Copy link
Member

Choose a reason for hiding this comment

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

스레드 풀은 종료하지 않아도 될까요? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 부분에 executorService.shutdown(); 를 추가하였습니다!!
생각해보니 과거 스레드가 남아있어 프로세스가 종료되지 않아 직접 작업관리자 or 커맨드로 제거해줬던 경험이 있는데 스레드풀도 마찬가지 겠군요...!

여기서 shutdownNow 메서드와 close 메서드가 보여 비교해본 결과!
전자는 대기 중인 작업을 취소하고, 실행 중인 작업을 가능한 한 중단하려 하는 것이고, try-with-resources 와 같은 구문 내에서 사용되며 내부적으론 shutdown 메서드 처럼 작동하는 것을 기대한다 합니다.

import org.junit.jupiter.api.Test;
import support.TestHttpUtils;

class ConnectorTest {
Copy link
Member

Choose a reason for hiding this comment

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

헉 테스트까지 작성하셨네요. 역시 고수시다. . .

Copy link
Member Author

Choose a reason for hiding this comment

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

동시성 테스트 첫 시도라 아직 많이 어렵네요... 잘모르겠어요 😥

Comment on lines 29 to 34
long doneCount = futures.stream()
.filter(Future::isDone)
.count();

// then
System.out.println("doneCount = " + doneCount);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
long doneCount = futures.stream()
.filter(Future::isDone)
.count();
// then
System.out.println("doneCount = " + doneCount);
// then
long doneCount = futures.stream()
.filter(Future::isDone)
.count();
assertThat(doneCount).isEqualTo(threadCount);

아무래도 콘솔에 출력하는 게 친근하죠. ㅎ ㅎ
그러나 테스트 코드니까 이렇게 검증문을 넣을 수도 있지 않을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

사실 마지막날 해당 메서드 하나 작성해보려고 몇 시간 날렸는데 count 가 조금 맞지 않더군요...
아직 제 머리론 동시성 테스트 작성하긴 이른 것 같아요 😅
똑같이 작성해서 테스트는 통과하는데 왜 그런지도 사실 잘 모르겠답니다.. 하하
알려주세요 리뷰어님 😃

Comment on lines 23 to 27
// when
for (int i = 0; i < threadCount; i++) {
futures.add(pool.submit(() -> TestHttpUtils.send("/index.html")));
}
pool.awaitTermination(10, TimeUnit.SECONDS);
Copy link
Member

Choose a reason for hiding this comment

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

잘 모르지만... ㅎ ㅎ 예상치 못한 작업이 제출되는 것을 막기 위해
awaitTermination()을 호출하기 전에 shutdown()을 해줘야 할 것 같은데, 테스트 코드라 상관없을까요?
새양은 어떻게 생각하시나요?!

Copy link
Member Author

Choose a reason for hiding this comment

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

음! 힌트를 얻어 추가로 더 정확하게 작성해 보았어요~

스레드풀의 submit 함수와 Future 그리고 shutdown 까지 학습이 부족했던 것 이었네요!
결국 테스트 코드를 작성했고 600개 요청 중 기대했던 350개의 요청만 성공하는 기대 값을 얻을 수 있었어요!!

Copy link
Member

Choose a reason for hiding this comment

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

새양이 실행했을 때는 항상 통과하던가요?
제가 할 때는 실행할 때마다 결과가 달라져서 대부분 실패하다가 가끔 성공해요. 😿
도움을 드리고 싶지만, 저도 테스트 작성에 어려움을 느끼고 있어서 도와드리지 못하네요.
죄송합니다 새양. 나중에라도 깨달은 바가 있다면 공유하도록 하겠습니다. ㅜ_ㅜ

public class SessionManager implements Manager {

private static final Map<String, HttpSession> SESSIONS = new HashMap<>();
private static final Map<String, HttpSession> SESSIONS = new ConcurrentHashMap<>();
Copy link
Member

Choose a reason for hiding this comment

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

ConcurrentHashMap과 HashTable의 차이가 뭘까요? ㅎ ㅎ

참고) https://tecoble.techcourse.co.kr/post/2021-11-26-hashmap-hashtable-concurrenthashmap/

Copy link
Member Author

Choose a reason for hiding this comment

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

HashMap 과 다르게 둘 다 멀티 스레드 환경에서 동시성을 보장해주지만 스레드간 동기화 락을 거는 HashMap 은 다소 느릴 수 있다는 단점이 존재했습니다.

ConcurrentHashMap 은 조작 하려는 특정 Entry 에 대해서만 락을 걸기 때문에 멀티 스레드에서의 성능을 향상 시킨다고 하네요!

간단 명료한 정리 글도 읽기 정말 좋군요! 감사합니다~

@geoje
Copy link
Member Author

geoje commented Sep 16, 2024

냥인~ 추석 잘 보내고 계신가요! 쉬는 날인데도 리뷰 달아주셨는데 응답이 다소 늦었네요 😅
말씀해주신 것 반영하고 키워드 많이 주셔서 이번에도 정말 많은 학습을 하고 가네요!!
역시 직접 찾아보는게 머리속에 잘 넣어지는 것 같습니다!!
감사합니다~

Copy link
Member

@cutehumanS2 cutehumanS2 left a comment

Choose a reason for hiding this comment

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

새양 ~ 마지막까지 저와 함께 하시느라 고생 많이 하셨습니다.
제가 더 많이 배워가는 시간이었어요. 감사합니다. ㅎ ㅎ

이번 미션에서 특히 학습에는 끝이 없다는 걸 느꼈어요.
새양과 더 공부하고 싶지만(ㅋㅋ) 다음 미션을 위해 이만 머지하겠습니다.
즐거운 연휴 보내십셔~~ 👋

@cutehumanS2 cutehumanS2 merged commit a777985 into woowacourse:geoje Sep 16, 2024
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

Successfully merging this pull request may close these issues.

2 participants