INTERNAL: Remove transcoder service logic in bulkGet apis.

[brido4125]

변경 사항

asyncGetBulk, asyncGetsBulk 연산에서 TranscoderService를 사용하지 않고
decode()를 WAS의 worker thread 수행하도록 변경하였습니다.

기존 테스트 코드에서는 테스트 코드의 메인 쓰레드가 decode될 동안 Blocking이 되어 IO 스레드로부터예외인 ExecutionException e이 발생하였고,
현재 구현에서는 테스트 메인 쓰레드가 Decode 작업을 수행하여 AssertionError가 발생하기 때문에 Throwable를 인자로 받도록 변경하였습니다.

[uhm0311]

TestWithKeyTranscoder 클래스가 어떤 역할을 하는 클래스이고, 왜 AssertError가 발생하는지 설명 바랍니다.

그리고 단순히 Throwable로 퉁치는 것은 AssertError가 아닌 다른 종류의 Exception(예를 들면 NullPointerException)이 발생했을 때 실제로는 테스트 코드가 실패한 것이지만 Throwable은 모든 Exception과 Error를 포함하는 타입이므로 테스트 성공으로 간주될 수 있습니다.
어떤 종류의 AssertError가 발생하는지 확인하고, 최대한 자세한 타입으로 변경하도록 합시다.

[brido4125]

TestWithKeyTranscoder는 생성자의 인자로 들어온 String값과 실제 저장하려는 String 데이터의 값을 합쳐 encode() 로직 수행 시 CachedData를 생성합니다.
즉, Decode 시에도 동일한 생성자의 인자를 가지지 않으면 현재 테스트 코드와 같이 Assertion 에러가 발생하게 됩니다.

아래 로직에서는 set 될 때 사용되는 tc 생성자의 String 인자 값과 getBulk를 수행할때 사용되는 tc의 생성자의 String 인자 값이 서로 다릅니다.
즉, encode 시 생성되는 CachedData의 data와 decode 시 확인하는 데이터가 달라 AssertionFailedError가 발생합니다.

public void testAsyncGetBulkWithTranscoderIterator() throws Exception {
    //...
    client.set(keys.get(0), 5, "val4", encodeTranscoder).get();
    Transcoder<String> decodeTranscoder = new TestWithKeyTranscoder("not " + keys.get(0));
    tcs.add(0, decodeTranscoder);
    try {
      client.asyncGetBulk(keys, tcs.listIterator()).get();
      fail("Expected ExecutionException caused by key mismatch");
    } catch (Throwable t) {
      // pass
    }
  }

말씀해주신 것처럼 Throwable => AssertionFailedError로 변경하겠습니다.

[jhpark816]

리뷰 완료

[jhpark816]

리뷰 완료

[jhpark816]

@brido4125 수정 중인가요?

[brido4125]

넵 완료되면 노티 드리겠습니다.

[jhpark816]

@brido4125
여기 수정은 tcService 제거와 무관하지 않나요?

[uhm0311]

#710 (comment)

위 코멘트 참조 바랍니다.

[jhpark816]

#710 (comment)

위 코멘트 설명이 관련되어 있는 것 같으면서도 조금 다른 것 같습니다.
ComparisonFailure에 관한 언급은 없는 것 같습니다.

기존 테스트에 어떤 문제가 있어서 수정하는 것인가요?
아니면 이번 수정에 의하여 무언가 변화가 있었던 것인가요?

[uhm0311]

#710 (comment)

ComparisonFailure는 위 코멘트에 따라 변경되었습니다.
기존 테스트 코드와 다른 에러가 발생하여 수정한 것입니다.

[jhpark816]

기존 unit testing에 문제가 없는 상태로 알고 있고,
이번 tcService 제거하는 PR에 의해 기존 테스트가 실패하게 된 것인가요?

[uhm0311]

tcService 제거하는 PR에 의해 기존 테스트가 실패하게 된 것인가요?

네 맞습니다.

[jhpark816]

@uhm0311 @brido4125
어떤 내용인지 확인했고, 대략 아래와 같이 이해합니다.

• 예전엔 tcService thread에 의해 decoding되고 AssertionError 발생하면
  ExecutionException 던진 것으로 예상되고,
• 현재는 worker thread가 직접 decoding하기 때문에 AssertionError를 직접 받게 되는 것이군요.

아래는 assertEuqals() 설명 내용입니다.
여기에 나온대로 ComparisonFailure 대신에 AssertionError를 사용합시다.
AssertionError 사용해야 이해하기가 쉽습니다.

#############

assertEquals
public static void assertEquals(String message,
long expected,
long actual)
Asserts that two longs are equal. If they are not, an AssertionError is thrown with the given message.

Parameters:
message - the identifying message for the AssertionError (null okay)
expected - long expected value.
actual - long actual value

[oliviarla]

@jhpark816 @uhm0311 @brido4125
TranscodeService의 필요성에 대해 예전에 정리해둔 내용입니다.

• key-value 에 저장된 데이터는 1MB까지도 저장되어 용량이 커 decode가 오래걸린다.
• compression 되어있는 경우에는 압축 해제까지 필요하다.

asyncBulkGet() API를 사용할 때 for문을 돌면서 decode작업을 순차적으로 진행하게 되는데, 위와 같이 용량이 크거나 압축 해제하는 작업이 발생하면 각각 스레드에 맡기는것보다 오래걸릴 수 있을 것 같은데 문제가 발생하진 않을까요?

[brido4125]

@oliviarla
WAS 환경이 spring 컨테이너라 가정하겠습니다.

기존 tcService의 thread들이 수행 -> tomcat의 쓰레드풀의 스레드가 수행으로
변경되었는데, 디코딩 작업 시간이 어떤 근거에서 증가할 수 있는지 설명 좀 부탁드리겠습니다.

제가 잘못 이해한 부분이 있네요 코멘트 수정해서 올려드릴게요

[brido4125]

@oliviarla

asyncGets가 아닌 asyncBulkGets 또는 asyncBulkGet 에서의 상황을 말씀하신거죠?
맞다면 코멘트 수정 부탁드려요

[oliviarla]

아 맞습니다. gets를 헷갈렸네요. asyncBulkGet 이 맞습니다.
기존 tcService를 쓰면 여러 데이터를 순차적으로 decode하는 것이 아니라, 스레드풀의 스레드에 태스크를 맡기고 비동기적으로 여러 데이터를 decode할 수 있는 것이 맞나요?
만약 그렇다면 tomcat 스레드풀의 스레드가 지금과 같이 for문을 돌며 여러 데이터를 decode하게 되면 동기와 비슷한 형태가 되지 않을까 싶어서요.

[brido4125]

기존 tcService를 쓰면 여러 데이터를 순차적으로 decode하는 것이 아니라, 스레드풀의 스레드에 태스크를 맡기고 비동기적으로 여러 데이터를 decode할 수 있는 것이 맞나요?

스레드풀을 이용하니까 동시에 여러 decode 작업이 가능합니다.

제 의견은 다음과 같습니다.

bulkGet과 같은 요청이 만약 100개가 넘어가게 온다면,
현재 쓰레드풀의 수(10)으로는 한번에 감당하지 못하고 대기가 발생하게 됩니다.
이 상황에서 asyngGet 요청이 발생하여 큐에서 디코딩 작업이 대기하면
해당 연산 자체가 타임아웃이 발생할 확률이 증가합니다.

TcService를 사용하지 않으면 말씀하것처럼 bulk 연산의 효율은 떨어지지만,
최소한 bulk 요청을 진행 중일 때, 들어온 asyncGet 요청이
타임아웃이 발생할 확률은 훨씬 줄일 수 있다고 볼 수 있습니다.

즉, 자주 사용되지 않는 연산이 늦어지는것과 자주 사용되는 연산의 타임아웃이 발생하는 것 중
선택해야한다면 전자를 선택해야하지 않나 생각합니다.

그러면 bulk 연산을 위해 tcService의 쓰레드풀의 수를 늘리는게 어떠냐? 라는 반문의 경우
자주 사용되지 않는 스레드를 항시 유지하는 리소스 또한 크기 때문에 추천되지 않는다고 생각합니다.
아무래도 asyncGet 연산이 asyncBulkGet 보다는 많이 사용될테니까요

[oliviarla]

나중에 사용자가 직접 Executor를 만들어 인자로 입력하는 API 제공하는 등 더 나은 방법을 논의하면 좋을 것 같습니다.

[jhpark816]

@oliviarla @brido4125 @uhm0311
OK. 이에 대해 별도로 offline 논의하시죠.