INTERNAL: Wait till the cmdlog flush thread ends

[ing-eoking]

🔗 Related Issue

• jam2in/arcus-works#559

⌨️ What I did

• cmdlog stop 시 flush thread의 sleep을 깨우고, 해당 thread가 종료될 때 cmdlog start가 불가하도록 변경

[ing-eoking]

~cmdlog stop과 cmdlog start가 짧은 시간에 이루어질 시 old cmdlog flush thread는 정상 종료하도록 변경~ 무조건 하나의 cmdlog flush thread만 구동되도록 변경

[jhpark816]

@uhm0311 추가 리뷰어로 지정했습니다.

[ing-eoking]

@namsic

pthread_join()을 호출하면 cmdlog thread가 종료될 때까지 대기할텐데, 이 시간은 항상 짧은 시간인가요?

모종의 이유로 스레드 종료가 지연되면, cmdlog stop 명령이 그 자체로 long query가 되어 다른 요청의 처리에 영향을 줄 가능성은 없나요?

2가지 상황이 있다고 생각됩니다. cmdlog thread는 4KB 이상의 데이터가 쌓이지 않으면, 5ms 동안 sleep을 하게 됩니다. 이 경우를 위해서 do_cmdlog_flush_wakeup 을 수행시키게 했습니다.

다음은 cmdlog buffer에 쌓인 data 크기가 너무 큰 경우 입니다. 이 경우 4KB 단위로 쪼개어 파일에 쓰게 됩니다. 이 경우는 long query가 될 수 있습니다.

[ing-eoking]

@namsic

기존 방식에서는 cmdlog thread를 다중으로 돌릴 경우 Data buffer 충돌을 피할 수 없습니다. Data Buffer를 2개(new, old)로 구성하거나, Head Tail 초기화 방식을 변경해야 합니다.

long query 상황이 적을 것이라 생각되나 (Memory 스펙과 Disk 스펙이 의해 달라지나 8KB 이상 쌓이는 현상이 적을 것으로 보입니다.) 먼저 CMD Log Buffer를 변경하는 방식으로 PR을 올릴까요?

그러면 아래의 이슈에서 해결방안 3번째으로 변경됩니다.

• jam2in/arcus-works#559

[namsic]

old, new thread를 구분하여 old thread가 종료되도록 하는 것보다, 어느 한 시점에는 하나의 thread만 존재하도록 만드는 것이 좋겠습니다.

즉, cmdlog stop 수행한 뒤에도 기존 thread가 완전히 종료되기 전까지는 cmdlog start 명령을 실패 처리해야 할 것으로 보입니다.

[ing-eoking]

@namsic

즉, cmdlog stop 수행한 뒤에도 기존 thread가 완전히 종료되기 전까지는 cmdlog start 명령을 실패 처리해야 할 것으로 보입니다.

@uhm0311 님과 같은 의견으로 보입니다.

그럴 경우 관리자가 cmdlog start의 시작 지점을 알 수 없게 됩니다. 현재는 cmdlog가 이미 켜져 있거나 파일 생성 실패에 대해서만 cmdlog start가 실패하며, 이는 에러 원인이 해당 cmdlog가 앞으로도 불가함이 명확합니다. 하지만 기존 thread가 종료가 되지 않아 에러 메시지를 날릴 경우 관리자는 지속적으로 cmdlog start를 날려 켜짐을 확인해야 합니다.

그리고 old thread가 남게 되는 이유는 써야할 메시지를 마치지 못해서입니다. 이는 자신의 메시지를 다 쓰게 되면 종료될 것이며, 결과적으로는 new thread만 남게됩니다. 두 thread간에 로그 파일이 구분되며, 공유되는 buffer 내에서도 write 할 위치 또한 다르므로 thread 간의 원자성을 꼭 가져가야할 이유는 없습니다.

기존 코드에서는 비교적 lock(buffer, flush, cmdlog)을 여러 개 생성했으며, 로직 구조 또한 cost 최적화를 위한 구조로 보입니다. 저는 우선 이 의도를 따라가는 것이 낫다고 생각하며, 만약 한 thread만 남긴다면, 불필요한 lock을 잡는 것에 대한 재검토가 필요합니다.

[ing-eoking]

코드 및 상단부 내용 수정되었습니다. 불필요한 lock 제거는 추후에 검토 후 올리겠습니다.

[jhpark816]

@namsic 최종 리뷰 바랍니다.