Skip to content

[톰캣 구현하기 4단계] 호기(조호연) 미션 제출합니다.#728

Merged
HyungHoKim00 merged 11 commits intowoowacourse:hoyeonyyfrom
hoyeonyy:step4
Sep 19, 2024
Merged

[톰캣 구현하기 4단계] 호기(조호연) 미션 제출합니다.#728
HyungHoKim00 merged 11 commits intowoowacourse:hoyeonyyfrom
hoyeonyy:step4

Conversation

@hoyeonyy
Copy link

안녕하세요 명오!

4단계 구현해서 제출합니다!

Copy link
Member

@HyungHoKim00 HyungHoKim00 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 호기!! 요청 드린 변경사항도 많은데, 잘 적용해주신 것 같아요.
질문 남겨 놓았으니 확인해주세요~

}
var processor = new Http11Processor(connection);
new Thread(processor).start();
executorService.submit(() -> new Http11Processor(connection).run());
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
executorService.submit(() -> new Http11Processor(connection).run());
executorService.submit(new Http11Processor(connection));

Runnable을 인자로 받고 있어서, 위와 같이 수정해도 똑같이 동작해요~

Copy link
Member

Choose a reason for hiding this comment

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

Future<?> 객체를 반환하는 submit() 함수를 사용하셨는데, 작업 결과를 이용하는 부분이 현재 코드에서는 없네요.
execute() 가 아닌 submit()을 선택하신 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

ExecutorService 스팩을 보니 submit 메서드가 정의되어 있어 사용했습니다..!
깊게 찾아보지 않고 사용한것 같아요,,!

반환값이 굳이 필요하지 않다면 execute를 사용한 것이 좋아보입니다!


private static final int DEFAULT_PORT = 8080;
private static final int DEFAULT_ACCEPT_COUNT = 100;
private static final int DEFAULT_MAX_THREADS = 250;
Copy link
Member

Choose a reason for hiding this comment

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

MAX_THREAD 값의 기준이 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

LMS 를 보고 따라쳤던게 끝이었던 것 같아요..!

명오 덕분에 톰켓 공식 문서에 들어가서 찾아봤습니다!

image

톰켓은 디폴트를 200개로 설정하고 있더라구요!

뿐만 아니라 스레드의 갯수를 어떤 지표를 보고 저울질을 해야할 까에 대한 공부를 했습니다!
cpu Bound, IO bound

registry.addResourceHandler(PREFIX_STATIC_RESOURCES + "/" + version.getVersion() + "/**")
.addResourceLocations("classpath:/static/")
.setCacheControl(CacheControl.maxAge(Duration.ofDays(365)).cachePublic());
.setCachePeriod(31536000)
Copy link
Member

Choose a reason for hiding this comment

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

setCachePeriod와 setCacheControl(CacheControl.maxAge)의 차이가 뭔가요?

Copy link
Author

Choose a reason for hiding this comment

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

사실 동작하는 방식은 같다고 알고 있습니다!

하지만 setCachePeriod는 단순히 기간만 설정한다면
setCacheControl은 다양한 캐시 옵션을 조합할 수 있다고 알고 있습니다! (예: public/private, must-revalidate)

accept-count: 1
max-connections: 1
accept-count: 0
max-connections: 2
Copy link
Member

Choose a reason for hiding this comment

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

이유는 모르겠으나, 현재 Apptest.test()와 FileTest.파일의_내용을_읽는다() 테스트가 둘 다 실패하고 있어요. 혹시 호기 환경에서 확인해주실 수 있나요?
image

Copy link
Author

Choose a reason for hiding this comment

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

image

아마 appTest 는 어플리케이션을 실행시키고 돌려야 돌아갈 것 같습니다!

}

public void matchService(HttpRequest request, HttpResponse response) throws Exception {
public void doService(HttpRequest request, HttpResponse response) throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

service() 라고만 작성하지 않고 do~를 붙이신 이유가 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

개인적으로는 메서드 이름이 중복 되는 것 같아 다르게 하자라는 마음밖에 없었던 것 같아요,,!

}

public boolean hasCss() {
public boolean isCss() {
Copy link
Member

Choose a reason for hiding this comment

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

Files.probeContentType()을 사용하면 파일의 content-type을 가져올 수 있습니다. Apache Tika라는 라이브러리도 존재해요~

body = "Hello world!";
statusLine = new StatusLine(protocol, "200", "OK");
httpHeader.putHeader(CONTENT_TYPE, TEXT_HTML_CHARSET_UTF_8);
statusLine = new StatusLine(protocol, Status.OK.getCode(), Status.OK.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

StatusLine이 String code, String message 필드가 아닌 Status 필드로 가지고 있는 건 어떤가요?

Copy link
Author

Choose a reason for hiding this comment

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

부랴부랴 리팩토링 하느랴 신경쓰지 못했는데 더 깔끔해진 것 같아요! 감사해요!

@@ -26,7 +26,7 @@ public String getMethodType() {
}
Copy link
Member

Choose a reason for hiding this comment

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

method(GET, POST 등)도 enum이면 좋을 것 같아요~

public Connector(final int port, final int acceptCount) {
public Connector(final int port, final int acceptCount, final int maxThreads) {
this.serverSocket = createServerSocket(port, acceptCount);
this.executorService = Executors.newFixedThreadPool(maxThreads);
Copy link
Member

Choose a reason for hiding this comment

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

ThreadPool의 종류가 여러개인데, 그 중 fixedThreadPool을 사용하신 이유가 뭔가요?

Copy link
Author

Choose a reason for hiding this comment

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

image

개인적으로 정리해놓은 threadpool 의 종류들입니다!

현재는 비동기 작업이나, 정기적인 작업이 필요없다고 생각해서 가장 기본적인 fixedThreadPool 을 사용했습니다

@@ -0,0 +1,24 @@
package org.apache.coyote.http11.response;

public enum Status {
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

@HyungHoKim00 HyungHoKim00 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 호기~ 요청드린 수정사항도 잘 반영해주셔서 머지를 하려 했으나, 딱 한가지 마지막 요청사항이 존재합니다 ㅠㅠ.
빠르게 수정하실 수 있으니 수정해주시고 바로 리뷰요청 주세요~

private final String protocol;
private final String statusCode;
private final String statusMessage;
// private final String statusCode;
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

@HyungHoKim00 HyungHoKim00 left a comment

Choose a reason for hiding this comment

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

다음 미션 파이팅입니다~

@HyungHoKim00 HyungHoKim00 merged commit 6a3a34d into woowacourse:hoyeonyy Sep 19, 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

Comments