Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

엔진 설정 파일로 구동한 경우에 중복 설정 처리 #454

Closed
SuhwanJang opened this issue Apr 9, 2020 · 11 comments
Closed

엔진 설정 파일로 구동한 경우에 중복 설정 처리 #454

SuhwanJang opened this issue Apr 9, 2020 · 11 comments
Assignees

Comments

@SuhwanJang
Copy link
Contributor

구동 옵션에 명시한 설정이 엔진 설정 파일에도 중복으로 있는 경우 엔진 설정 파일의 값으로 최종 설정된다.
구동 옵션에 명시한 설정이 최종 설정 되도록 변경한다.

-e config_file=path 를 구동 옵션 스트링에 append 하는데, engine 에서 차례로 읽어 반영하면 config_file 을 마지막에 읽기 때문에 구동 옵션이 overwrite 된다.

memcached.c

int main() {
         case ....
         case 'e':
             engine_config = optarg;
             break;
      }

     old_opts += sprintf(old_opts, "num_threads=%lu;", (unsigned long)settings.num_threads);
     if (settings.verbose) {
         old_opts += sprintf(old_opts, "verbose=%lu;", (unsigned long)settings.verbose);
     }
...
     if (engine_config != NULL && strlen(old_options) > 0) {
         /* If there is -e, just append it to the "old" options that we have
          * accumulated so far.
          */
         old_opts += sprintf(old_opts, "%s", engine_config);
         engine_config = NULL; /* So we set it to old_options below... */
         /*
         settings.extensions.logger->log(EXTENSION_LOG_WARNING, NULL,
                 "ERROR: You can't mix -e with the old options\n");
         return EX_USAGE;
         */
     }
     if (engine_config == NULL && strlen(old_options) > 0) {
         engine_config = old_options;
     }
     mc_logger->log(EXTENSION_LOG_INFO, NULL, "engine config: %s\n", engine_config);
}

해결 방안 :
append 하지 않고, 별도로 전달하여 처리.
init_engine(engine_handle, 구동 옵션, "-e config_file=path", mc_logger);

@MinWooJin 검토 요청드립니다.

@SuhwanJang SuhwanJang self-assigned this Apr 9, 2020
@jhpark816

This comment was marked as outdated.

@SuhwanJang

This comment was marked as outdated.

@SuhwanJang

This comment was marked as outdated.

@jhpark816
Copy link
Collaborator

@SuhwanJang
본 이슈가 문제되는 지를 다시 확인하고, 의견 주세요.

@SuhwanJang
Copy link
Contributor Author

SuhwanJang commented Dec 2, 2020

@jhpark816
엔진 설정 파일에는 구동 옵션으로 주는 설정이 중복으로 들어가지 않을 것이기 때문에 문제 없다고 봅니다.
close 해도 될 것 같습니다.

@jhpark816
Copy link
Collaborator

jhpark816 commented Dec 2, 2020

@SuhwanJang
구동 옵션이 config file에 중복으로 들어가면 config file 설정이 우선 순위를 가지게 되는 데,
이 경우 어떤 문제가 발생할 수 있나요 ?

@SuhwanJang
Copy link
Contributor Author

@jhpark816
문제는 없지만, 구동 옵션과 config file 중 어떤 것을 우선 순위를 둘 것인가에 따라서 의미상 안 맞는 부분이 생깁니다.
구동 옵션에 우선순위를 두도록 결정한다면, 최종 반영되는 설정은 현재 동작과 같이 config file 설정되는 방식을 수정을 해야 할 것입니다.
제 생각으로는 구동 옵션을 우선 순위를 두는 것이 사용 관점에서 좀 더 맞는 것 같고,
현재는 중복되는 옵션이 없는데 향후에 처리해도 되지 않나 싶습니다.

@jhpark816
Copy link
Collaborator

@SuhwanJang
향후에 처리할 예정이고, 현재는 아래 사항만 정리해 둘 예정입니다.
아래 사항에 대해 의견만 정리해 주세요.

  • 구동 옵션이 config file에 중복 설정될 경우의 문제점은 무엇인가 ?
  • 이러한 문제를 어떻게 처리할 것인가 ?

참고로, 현재 구동 옵션 중에 engine 설정으로 옮겨야 할 것도 있고,
실수로 engine conifg에 구동 옵션을 넣을 수도 있습니다.

@SuhwanJang
Copy link
Contributor Author

@jhpark816
구동 옵션이 config file에 중복 설정될 경우의 문제점

  • config_file string 은 구동 옵션 string 에 append 하여 엔진에 전달합니다. 이로 인해 config_file read 를 마지막에 수행하여 config_file 의 설정으로 최종 설정됩니다.

해결방안

  • config read 순서를 config file -> 구동옵션 순으로 할 수 있도록 main() 에서 구동 옵션 string 을 만들 때 config_file 을 맨 앞에 위치하게끔 만들면 됩니다.
  • 구현은 두 가지 방법으로 할 수 있는데,
    • 구동 옵션 string 에서 config file string 과 그 외에 옵션 string 을 따로 버퍼에 담았다가 합치는 방법.
      => 기존에 비해 추가 메모리 필요
    • 구동 옵션 버퍼에서 config file length (50 제한) 만큼 비워두고 구동 옵션을 넣는 방법.

@jhpark816
Copy link
Collaborator

@SuhwanJang 인턴 작업으로 진행합시다.

@jhpark816
Copy link
Collaborator

엔진 설정은 2가지 형태로 제공할 수 있습니다.

  • 일반 구동 옵션 (환경 변수 지정 옵션 포함)
  • 엔진 설정 옵션인 -e 옵션을 통한 config_file 설정

동일한 설정에 대해 2가지 방식으로 제공할 경우,
기존 방식대로 config_file 옵션이 우선순위를 가지도록 합니다.
향후, 일반 구동 옵션으로 지정한 엔진 설정을 config_file 설정으로 변경할 예정입니다.

engine config 정리 (#541) 이슈에서 다루고 있으므로,
본 이슈는 close 합니다.

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

No branches or pull requests

3 participants