INTERNAL: Move a cmdlog_in_use variable

[ing-eoking]

🔗 Related Issue

• jam2in/arcus-works#526

⌨️ What I did

• cmdlog_in_use 변수를 cmdlog 모듈로 이동합니다.

🤔 Others

• 간단하게 이동만 시켰습니다.
• 이외에도 내부에 두고 API로 노출시키는 방향도 있습니다.

[namsic]

in_use 변수에 대한 비교 로직을 모듈 내부로 이동시키는 것에 대해 의견 주시면 좋겠습니다.

예를 들어, cmdlog_write()를 아래와 같이 변경하면 어떤가 하는 것입니다.

-bool cmdlog_write()
+void cmdlog_write()
{
+   if (!cmdlog_in_use) return;

    /* . . . */

    if (! cmdlog.on_logging) {
-       return false;
+       cmdlog_in_use = false; return;
    }

    /* . . . */

-   return true;
}

call stack이 한 단계 늘어나는 문제가 있기는 하지만, 호출부의 구현을 훨씬 간결하게 유지할 수 있을 것입니다.

• 기존

  if (cmdlog_in_use) {
      if (cmdlog_write(c->client_ip, command) == false) {
          cmdlog_in_use = false;
      }
  }

• 변경

  cmdlog_write(c->client_ip, command)

[jhpark816]

@ing-eoking @namsic

command logging 관련하여 minor refactoring 진행하였으니, 참고 바랍니다.

• https://github.com/naver/arcus-memcached/commit/2e03bc7e52a842bc4e7ab48f6b81a52b268200b7

아래와 같은 변수가 있는 데, 구분하여 사용하는 것이 좋은 지도 확인하시고,

• cmdlog_in_use
• cmdlog.on_logging

cmdlog_in_use는 cmdlog 모듈 안에서 설정하고, extern 변수로 조회만 하게 만들면 좋겠습니다. 아래 경우만 사용될 것 같습니다.

    if (cmdlog_in_use) {
        if (cmdlog_write(c->client_ip, command) == false) {
            cmdlog_in_use = false;
        }
    }

함수 호출을 줄이기 위해 cmdlog_in_use 변수는 두는 것이 낫지 않나 생각합니다. 어떤가요? cmdlog_write() 함수를 항상 호출하고 그 안에서 검사한다면 cmdlog_in_use 자체가 필요 없지만 성능 상 불리할 수 있습니다.

[namsic]

함수 호출을 줄이기 위해 cmdlog_in_use 변수는 두는 것이 낫지 않나 생각합니다. 어떤가요? cmdlog_write() 함수를 항상 호출하고 그 안에서 검사한다면 cmdlog_in_use 자체가 필요 없지만 성능 상 불리할 수 있습니다.

약간의 성능 저하를 감수하더라도 cmdlog_in_use를 두지 않는 편이 코드 구조를 단순화하는 데 도움이 될 것 같아서 이전 코멘트를 남겼는데요. 함수 호출 비용이 성능에 얼마나 영향을 주는지까지 충분히 확인하고 의견을 냈던 것은 아니라서 성능 저하가 무시할 수 없는 수준이라면 cmdlog_in_use를 유지하는 것이 좋겠습니다.

[ing-eoking]

아래와 같은 변수가 있는 데, 구분하여 사용하는 것이 좋은 지도 확인하시고, cmdlog_in_use cmdlog.on_logging

둘은 같은 기능을 담당하는 변수이기에 cmdlog.on_logging을 cmdlog_in_use로 합치는 것이 좋을 것 같습니다. ~하지만 동시성에 의한 충돌 가능성이 있으므로 확인이 필요할 것 같습니다.~ cmdlog.on_logging에서 write는 모두 lock을 잡고 있는 것을 확인했습니다.

@jhpark816

cmdlog.on_logging 을 cmdlog_in_use로 변경하고, memcached.c에서는 read만 하면 될 것 같습니다.

[ing-eoking]

수정되었습니다.

[jhpark816]

@namsic 위의 2가지 사항에 대해 의견 한번 주시죠.

[jhpark816]

용어 변경은 다음에 다른 기능과 함께 다루도록 하겠습니다.