BulkGet 요청 처리 재검증

[jhpark816]

bulk get 수행 방식이 현재 최적인지 확인하고, 개선 부분을 도출한다. 특히, 아래 경우들을 고려한다.

• inactive node로 mapping되는 key들이 존재할 경우
• 해당 연산이 하나라도 오류가 발생할 경우
• 해당 연산이 하나라도 timeout 발생할 경우

추가적으로, local cache 활용도 정상적인지 확인한다.

[jhpark816]

@hjyun328 @sUpniverse 모두 확인해 주면 좋겠습니다.

[hjyun328]

BulkGetFuture - get

• 하나의 Key라도 Error, Cancel, Timeout이 발생된다면 Exception을 발생시킴.
• 모든 Key의 Operation이 성공적이어야 Key, Value 데이터들을 Map으로 반환함.

BulkGetFuture - getSome

• 하나의 Key라도 Error, Cancel이 발생된다면 Exception을 발생시킴.
• Error, Cancel 없이 Operation이 성공한 Key, Value 데이터들을 Map으로 반환. 반환된 Map에서 Timeout Key는 존재하지 않음.

local caching 동작은 정상인 것으로 보입니다. key list중 일부 key만 local cache에 있다면 서버로 요청되지 않습니다. 또한 local caching을 제외한 응답에 성공한 key들은 local cache에 저장됩니다.

[hjyun328]

@jhpark816 @sUpniverse

지금은 하나의 operation이라도 실패(operation error, inactive node)가 발생한다면, 일부 성공한 key의 결과를 받을 수 없는 문제가 있습니다.

BulkGetFuture에 새로운 API를 만들어서, 모든 Key에 대한 응답을 Map에 담아 리턴하면 어떨까 싶습니다. 예를 들어 성공한 Key는 Data를, 실패한 Key는 Exception을 반환하는 것으로 개선해볼 수 있을 것 같습니다.

class Result<T> {
  Exception e;    // Error, Cancel, Timeout Exception
  T t;           // data
}

public Map<String, Result<T>> get(long duration, TimeUnit units) {
  ...
}

또는 getSome을 수정해서 성공한 Operation만 Map에 담아 리턴해볼 수도 있을 것 같네요.

[jhpark816]

@sUpniverse 위의 코멘트 확인하고 의견 주세요.

[sUpniverse]

@jhpark816 @hjyun328 저도 형준 선임 의견 제안에 동의합니다. 현재는 all or nothing 형태로 캐시 조회가 이루어지고 이를 개선한 API가 getSome()으로 Exception 발생 전까지 일부 성공한 key에 대해 조회할 수 있습니다.

성공한 key의 결과만 받는것 보단, 모든 Key의 응답을 담는게 응용에서도 실패한 키에 대해서만 다시 재시도 하거나 DB에 일부 조회하는 패턴에 더 효율적 일것같습니다.

현재 getSome()을 이용한 조회 패턴은 자신이 가지고 있는 키 리스트에서 캐시 조회에 성공한 키를 제외한 실패 목록을 획득해야 하는 하나의 추가 과정이 필요합니다.

그렇기 때문에 전체 조회 결과를 들고오는 API가 더 응용의 관점에서는 편리하다고 생각합니다.

구현 관점에서는 모든 결과를 들고 오는 API를 get()으로 지원하는게 더 적절하다고 생각됩니다. 의미상으로는 현재 get()은 all or nothing으로 전체를 조회하는 get()의 의미와는 조금 다르다고 생각하는데이 부분은 조금 더 논의 하면 좋을것 같습니다.

[jhpark816]

@hjyun328 의견입니다.

• backward compatibility 지원해야 하므로, get(), getSome() 동작 변경은 어려울 것 같습니다.
• 어떤 새로운 API 제공할 지의 관점에서 정리하시죠. New API 반드시 지원해야 할 지 or 현재 API 정도로 무난할 지 의견도 들었으면 합니다.
  • 모든 결과 (실패 포함)
  • 성공한 결과만
  • 실패한 결과만
• BulkGetFuture 클래스에 대해 리팩토링 필요한 지 한번 봐 주세요.
• BulkGetFuture 클래스 질문
  • cancel 시에 WRITING 상태는 cancel 하지 않는 것이 맞는 건지?
  • cancel 시에 이미 읽은 연산을 cancel하는 것이 맞는 건지?

[oliviarla]

@jhpark816

어떤 새로운 API 제공할 지의 관점에서 정리하시죠. New API 반드시 지원해야 할 지 or 현재 API 정도로 무난할 지 의견도 들었으면 합니다.

• 현재 getbulk 역시 pipe와 같이 하나의 노드에 최대 200개씩 mget 연산을 분할해 보내게 됩니다. 현재와 같이 비동기로 보낼 경우, 어차피 연산을 모두 보낸 상태이기 때문에 모든 결과를 반환하는 것을 고려해보아야 합니다. 하지만 동기로 보낼 경우에는 현재의 getSome 메서드만 제공하더라도 문제가 되지 않을 것 같습니다.

BulkGetFuture 클래스 질문 cancel 시에 WRITING 상태는 cancel 하지 않는 것이 맞는 건지? cancel 시에 이미 읽은 연산을 cancel하는 것이 맞는 건지?

• 이 부분은 사용자가 Future의 cancel 메서드를 호출 할 때 어떻게 처리할지에 대한 것인가요?

[oliviarla]

@jhpark816 현재 asyncGetBulk 상태는 아래와 같습니다. getSome 메서드로 일부 데이터를 얻을 수 있는 경우는 타임아웃이 발생하면서 응답은 정상적인 상태인 경우밖에 없습니다.

• inactive node로 mapping되는 key들이 존재할 경우
  • 해당 key들에 대한 Operation은 cancel되기 때문에 결국 예외가 발생하게 되어 getSome으로도 조회에 성공한 데이터를 받아올 수 없다.
• 해당 연산이 하나라도 오류가 발생할 경우
  • get/getSome에서 모두 예외 발생하여 결과 조회 불가
• 해당 연산이 하나라도 timeout 발생할 경우
  • get에서는 예외 발생하여 결과 조회 불가
  • getSome에서는 예외 발생하지 않아 결과 조회 가능
• local cache 처리
  • 입력받은 모든 키를 순회하며 local cache에서 get을 해본다.
  • local cache에서 찾지 못한 데이터는 서버에서 가져온다.
  • get 메서드의 서버에서 가져오는 과정에서 error, cancel, timeout으로 인해 예외 발생 시 그대로 예외가 전파된다. getSome 메서드의 경우 timeout 발생 시 일부 데이터만 반환되고 이 데이터들은 local cache에 저장된다.
  • 예외가 발생하지 않았다면 서버에서 가져온 데이터를 local cache에 넣고 전체 결과를 반환한다.

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

• get 메서드의 경우 다수 캐시 노드에서 발생한 예외들을 복합 예외로 묶어 반환하여 사용자가 한 번에 예외 사항을 파악하기 쉽도록 한다.
• getSome 메서드 제공의 목적은 특정 노드에서 예외가 발생하여도 다른 노드들의 응답을 반환하는 것이 되어야 get 메서드와 차별화될 것이다. 이 방식을 제공하기 위해서는 getSome 메서드의 반환은 Map<String, T> succeed, Map<String, Exception> failed 필드를 담는 Result 클래스가 되어야 한다.

---

cancel 시에 WRITING 상태는 cancel 하지 않는 것이 맞는 건지? cancel 시에 이미 읽은 연산을 cancel하는 것이 맞는 건지?

현재 WRITING, READING 상태일 때에는 cancel에 따른 처리를 해주지 않고 있습니다.

WRITING 상태일 때에는 버퍼에 요청을 넣고 있으므로 cancel 처리를 하기가 애매합니다.

READING의 경우 TCP 응답이 왔을 때 처리해줄 Operation이 필요하므로 ReadQueue에 등록 해주어야 합니다. ~현재는 cancel된 Operation여도 readFromBuffer 메서드가 호출되어 데이터를 읽어들이고 있습니다. 제 생각에는 readFromBuffer 메서드 내부에서 isCancelled 메서드를 호출해 cancel된 상태이면 데이터를 읽지 않고 무시하면 될 것 같습니다.~

https://github.com/naver/arcus-java-client/blob/f60ed483c1d9ad76f5bbcaa1d8f17e1a294d6643/src/main/java/net/spy/memcached/protocol/ascii/OperationImpl.java#L143-L167

[jhpark816]

@oliviarla @uhm0311 의견입니다.

• get(), getSome() 메소드의 인터페이스는 그대로 유지
• getSome()에서 Error, Cancel 발생하더라도 조회가 완료된 결과를 리턴할 수 있도록 처리
• 조회 성공 결과와 조회 실패 결과(exception)를 모두 전달하고자 한다면, 별도의 getXXX() 메소드로 제공해야 할 것 같음. getSome() 만으로도 충분하지 않나 생각합니다.
• front caching 활용은 문제 없는 상태

[oliviarla]

@jhpark816

get 메서드의 경우 다수 캐시 노드에서 발생한 예외들을 복합 예외로 묶어 반환하여 사용자가 한 번에 예외 사항을 파악하기 쉽도록 한다.

이 부분은 적용하지 않나요?

[jhpark816]

@oliviarla

get 메서드의 경우 다수 캐시 노드에서 발생한 예외들을 복합 예외로 묶어 반환하여 사용자가 한 번에 예외 사항을 파악하기 쉽도록 한다.

이 부분은 적용합시다.

[oliviarla]

PR 머지되어 close합니다.