FEATURE: Refactor config file support for server configuration #945

ing-eoking · 2025-12-22T00:57:08Z

🔗 Related Issue

⌨️ What I did

기존에 engine 설정만 가능했던 config file 설정 기능을 확장하여, engine외의 다른 설정도 파일 형태로 가능하게 하도록 합니다.
(-e config_file=/config_file1.conf)

구동시 옵션을 직접 전달하는 대신, config file 경로를 인자로 넘겨 초기 설정을 자동으로 적용하도록 합니다.
(대부분의 프로그램에서 차용하는 방식)
- main()실행시, 구동옵션을 확인하기 전에 첫번째 인자를 config file경로를 인식하도록 기능을 확장합니다.

./memcached /path/to/arcus.conf

try_load_config_file()을 구현하여, .conf파일을 라인 단위로 읽어서 모든 값을 설정합니다.
- getopt의 optind를 동적으로 조정하여 config file과 구동옵션을 혼용할때, 구동 옵션값이 덮어씌어지도록 합니다.
https://github.com/jam2in/arcus-works/issues/808 를 통한 old_opts 중복 해결
- 기존에는 getopt(구동옵션값 설정)루프 내에서 settings 전역 구조체와 old_opts 문자열 생성을 동시에 수행하기에, config file과 구동옵션을 동시에 수행하면 old_opts에 중복된 값이 들어갔습니다.
- 이에 config_file과 구동옵션 모두 settings 전역 구조체만 업데이트를 수행하고, 파싱이후에 old_opts 문자열을 작성하도록 했습니다.

jhpark816

리뷰 완료

memcached.c

jhpark816 · 2025-12-26T09:18:43Z

memcached.c

+            settings.username = strdup(optarg);
            break;
        case 'P':
+            FREE_CFGFILE_VAL(settings.pid_file);


FREE_CFGFILE_VAL() 매크로가 필요한 이유가 이해되지 않는 데, 설명 바랍니다.
참고로, 위의 settings.username 처럼 free 하면 될 것 같습니다.

참고로, 위의 settings.username 처럼 free 하면 될 것 같습니다.

기존에 optarg 그대로 사용하는 대신 strdup()으로 메모리 할당받도록 변경했는데,
종료할 때 settings.username free 하는 로직 빠진 것으로 보입니다.

FREE_CFGFILE_VAL() 매크로가 필요한 이유가 이해되지 않는 데, 설명 바랍니다.
참고로, 위의 settings.username 처럼 free 하면 될 것 같습니다.

같은 구동 옵션이 여러 번 전달되는 경우를 처리하기 위한 코드입니다.
config 파일 기능을 사용하는 경우 settings.pid_file에는 메모리 할당된 값이 저장되지만, 기존 구동 옵션을 사용하는 경우에는 optarg의 주소를 그대로 가리키게 됩니다.
이 상태에서 동일한 구동 옵션을 여러 번 사용하면(memcached -P ./pidfile1 -P ./pidfile2 ...), 해당 값을 free하면 안 되는 상황이 발생하므로 이를 방지하기 위해 위의 매크로가 필요합니다.

기존에 optarg 그대로 사용하는 대신 strdup()으로 메모리 할당받도록 변경했는데,
종료할 때 settings.username free 하는 로직 빠진 것으로 보입니다.

settings.username만 아닌 arcus_proxy_cfg 쪽도 빠져있습니다. 현재 PR에서는 이를 별도로 처리하지 않습니다.

아래 두 가지 패턴을 사용하고 있는데, 구분 기준은 무엇인가요?

FREE_CFGFILE_VAL 사용하는 경우

strdup + free 사용하는 경우

아래 두 가지 패턴을 사용하고 있는데, 구분 기준은 무엇인가요?

REFACTOR: 'main' local variables to 'settings' struct #944 (comment)

정확한 사항은 모릅니다.
외부 API로 해당 주소 공간을 넘기는 경우, strdup을 쓴 것으로 추정하고 있습니다.

이전 코멘트에서 질문을 모호하게 했는데, 기존 구현과 별개로
이번 PR에서 optarg를 그대로 사용하던 옵션은 모두 FREE_CFGFILE_VAL으로 변경하였는데
username만 예외로 strdup() 사용한 이유를 물으려던 것이었습니다.

username 쪽에 strdup 처리가 들어가 있는 지 몰랐습니다.
현재는 수정 완료되었습니다.

memcached.c

jhpark816 · 2025-12-31T07:07:06Z

@ing-eoking
draft로 변경한 이유가 있나요?

ing-eoking · 2025-12-31T07:24:13Z

@jhpark816

INTERNAL: Change evict_to_free/detail_enabled to boolean #949

PR변경이 있어서 잠시 draft로 두었습니다

draft 해제했습니다.

jhpark816

리뷰 완료

memcached.c

jhpark816 · 2025-12-31T10:36:50Z

memcached.c

    }
 }

+#define EXTENSION_PATH_MAX_SIZE 10


이 값은 5이어도 충분할 것 같습니다.

jhpark816 · 2025-12-31T10:41:44Z

memcached.c

+
+static int load_config_file(const char *path)
+{
+    int ret = EXIT_SUCCESS;


EXIT_XXXX 코드와 EX_XXXX 코드가 혼용되어 사용되는 데요.
성공하면 0, 실패하면 -1을 리턴하는 것이 간단할 것 같습니다.

binding protocol 에러의 경우 EX_USAGE, 그 외의 경우는 EXIT_FAILURE로 종료하고 있습니다.

binding protocol에러는 -2, 그 외의 경우는 -1을 반환하도록 변경하겠습니다.

jhpark816 · 2025-12-31T10:45:59Z

memcached.c

+    uint32_t cache_memory_limit = settings.maxbytes / (1024 * 1024);
+#ifdef ENABLE_STICKY_ITEM
+    uint32_t sticky_memory_limit = settings.sticky_limit / (1024 * 1024);
+#endif


cache_memory_limit과 sticky_memory_limit은 0을 초기 값으로 가져도 될 것 같습니다.

cache_memory_limit와 sticky_memory_limit 값을 0으로 설정하면,
config 파일에서 해당 값들이 별도로 할당되지 않더라도 기본값이 아닌 0으로 바뀌게 됩니다.

기본값을 아래처럼 명시적으로 설정할 수도 있지만, 혹시 향후 기본값이 변경될 가능성을 고려하여 현재와 같이 구현했습니다.

uint32_t cache_memory_limit = 64; #ifdef ENABLE_STICKY_ITEM uint32_t sticky_memory_limit = 0; #endif

다른 형태를 원하신다면 아래 형태도 가능합니다.

#define DEFAULT_MEMORY_LIMIT 64 #define DEFAULT_STICKY_LIMIT 0 ... static void settings_init(void) { ... settings.maxbytes = DEFAULT_MEMORY_LIMIT * 1024 * 1024; settings.sticky_limit = DEFAULT_STICKY_LIMIT * 1024 * 1024; } static int load_config_file(const char *path) { uint32_t cache_memory_limit = DEFAULT_MEMORY_LIMIT; #ifdef ENABLE_STICKY_ITEM uint32_t sticky_memory_limit = DEFAULT_STICKY_LIMIT; #endif }

cache_memory_limit와 sticky_memory_limit는 로컬 변수인 데,
이에 대해 특별한 초기 값을 설정하는 것 보다 아래 형태의 코드가 좋겠습니다.

static int load_config_file(const char *path) { uint32_t cache_memory_limit = 0; #ifdef ENABLE_STICKY_ITEM uint32_t sticky_memory_limit = 0; #endif . . . if (cache_memory_limit > 0) { settings.maxbytes = (size_t)cache_memory_limit * 1024 * 1024; } #ifdef ENABLE_STICKY_ITEM if (sticky_memory_limit > 0) { settings.sticky_limit = (size_t)sticky_memory_limit * 1024 * 1024; } #endif }

memcached.c

jhpark816

일부 리뷰

memcached.c

jhpark816 · 2026-01-02T03:42:45Z

arcus.conf

+# -----------------------------------------------------------------------------
+# Maximum memory to use for item storage. (Default: 64M)
+# Use suffixes: K, M, G, T (case-insensitive)
+memory_limit=64M


여기는 단위가 포함됩니다.

반면, 구동 옵션으로 읽을 때는 단위를 MB로 인식하고 있는 데, 이 부분이 이상한 것이군요.
여기 단위를 포함하여 읽는다면, settings.maxbytes로 바로 읽어 들여도 될 것 같습니다.

sticky_limit 지정 시에도 단위를 지정하는 것으로 하구요.

@ing-eoking @namsic
여기 설정과 아래 load_config_file() 코드를 보면,
이렇게 설정한 것으로 테스트를 한번도 안 해 본 것 같은데요. 어떤가요?

memory_limit에 대한 설명이 해당 내용으로 쓰여있는 지 몰랐습니다.

현재 테스트는 내부 값까지 모두 검증하고 있지는 않고,
설정 파일로 동작시킨 뒤 관련 명령어가 정상적으로 동작하는지 정도만 확인하고 있습니다.
이후 테스트는 stats settings를 통해 출력된 값들이 실제 설정 값과 일치하는지까지 확인하도록 테스트를 보완하겠습니다.

기존 구동 옵션 및 config 명령어를 통해 설정할 수 있는 memory_limit과 sticky_limit 값이 모두 MB 단위이기 때문에,
현재 구현 방식대로 동작하는 것이 맞다고 생각되며, 설명을 sticky_limit과 동일하게 변경하는 편이 좋을 것으로 생각됩니다.

# Sticky (gummed) memory limit in MB. (Default: 0 - Unused) # Reserved memory for items that should not be evicted. # sticky_limit=0

한편 config_parser에서는 기본적으로 단위(k, m, g, t)를 지원하고 있어, 숫자 형태의 값을 읽어들일 때 단위를 함께 읽는 것이 강제됩니다. 이 경우 예를 들어 64M를 설정하면 실제 내부 값은 64T로 해석되어 사용됩니다.

cache_size=70368744177664

memory_limit 값을 MB 단위(1024 * 1024)로 변환하지 않고 숫자 그대로 처리하도록 구현할 경우, 설정 파일에 1024 * 1024보다 작은 값을 지정하면 내부적으로는 해당 값이 그대로 설정됩니다.
하지만 config 명령어에서는 값을 1024 * 1024로 나누어 출력하기 때문에, 결과적으로 0으로 표시되는 문제가 발생하게 됩니다.

maxbytes 설정의 최종 방안을 결정하여 알려주세요.

config 파일에서 MB 단위로 값을 지정하지 않은 경우, Warning 성격의 로그 메시지만 남기도록 변경했습니다.

jhpark816

리뷰 완료

memcached.c

jhpark816 · 2026-01-07T02:53:47Z

@namsic 최종 리뷰만 한번 해 주세요.

jhpark816 · 2026-01-07T05:15:42Z

@ing-eoking
수고했습니다.

ing-eoking marked this pull request as draft December 22, 2025 00:57

ing-eoking mentioned this pull request Dec 22, 2025

CLEANUP: Validate options after parsing all options #946

Merged

ing-eoking marked this pull request as ready for review December 26, 2025 08:23

ing-eoking force-pushed the singsangssong/configfile branch 2 times, most recently from a20ccb4 to bd65692 Compare December 26, 2025 08:29

jhpark816 reviewed Dec 26, 2025

View reviewed changes

namsic reviewed Dec 26, 2025

View reviewed changes

memcached.c Outdated Show resolved Hide resolved

memcached.c Outdated Show resolved Hide resolved

ing-eoking force-pushed the singsangssong/configfile branch 3 times, most recently from 7faba24 to b2570e2 Compare December 29, 2025 01:07

jhpark816 requested a review from namsic December 29, 2025 03:59

ing-eoking force-pushed the singsangssong/configfile branch 9 times, most recently from 38b8dbe to 7c5e437 Compare December 30, 2025 00:52

namsic approved these changes Dec 30, 2025

View reviewed changes

ing-eoking requested a review from jhpark816 December 31, 2025 04:52

ing-eoking mentioned this pull request Dec 31, 2025

INTERNAL: Change evict_to_free/detail_enabled to boolean #949

Merged

ing-eoking marked this pull request as draft December 31, 2025 05:53

ing-eoking force-pushed the singsangssong/configfile branch from 7c5e437 to b0f4957 Compare December 31, 2025 06:53

ing-eoking marked this pull request as ready for review December 31, 2025 07:26

jhpark816 reviewed Dec 31, 2025

View reviewed changes

ing-eoking force-pushed the singsangssong/configfile branch from b0f4957 to d4efac0 Compare January 2, 2026 00:39

ing-eoking force-pushed the singsangssong/configfile branch from 4e4d057 to a7f7913 Compare January 2, 2026 03:04

jhpark816 reviewed Jan 2, 2026

View reviewed changes

memcached.c Outdated Show resolved Hide resolved

memcached.c Outdated Show resolved Hide resolved

memcached.c Outdated Show resolved Hide resolved

ing-eoking force-pushed the singsangssong/configfile branch from a7f7913 to c245ed4 Compare January 2, 2026 03:22

jhpark816 reviewed Jan 2, 2026

View reviewed changes

ing-eoking force-pushed the singsangssong/configfile branch 4 times, most recently from c35f177 to 622d768 Compare January 5, 2026 08:24

ing-eoking marked this pull request as draft January 5, 2026 08:27

ing-eoking marked this pull request as ready for review January 5, 2026 08:37

ing-eoking mentioned this pull request Jan 6, 2026

INTERNAL: Move extension config extraction into load_extension #952

Merged

ing-eoking force-pushed the singsangssong/configfile branch 3 times, most recently from 60c3cb6 to 7dcf3be Compare January 6, 2026 02:45

ing-eoking mentioned this pull request Jan 6, 2026

INTERNAL: Add a callback type for config_parser #953

Merged

jhpark816 reviewed Jan 6, 2026

View reviewed changes

memcached.c Outdated Show resolved Hide resolved

memcached.c Outdated Show resolved Hide resolved

memcached.c Outdated Show resolved Hide resolved

ing-eoking marked this pull request as draft January 6, 2026 07:33

ing-eoking force-pushed the singsangssong/configfile branch 2 times, most recently from a7fda32 to 2719eb8 Compare January 7, 2026 02:05

FEATURE: Refactor config file support for server configuration

0ec139c

ing-eoking force-pushed the singsangssong/configfile branch from 2719eb8 to 0ec139c Compare January 7, 2026 02:06

ing-eoking marked this pull request as ready for review January 7, 2026 02:06

ing-eoking requested review from jhpark816 and namsic January 7, 2026 02:12

namsic approved these changes Jan 7, 2026

View reviewed changes

jhpark816 approved these changes Jan 7, 2026

View reviewed changes

jhpark816 merged commit 7e3e541 into develop Jan 7, 2026
1 check passed

ing-eoking deleted the singsangssong/configfile branch January 7, 2026 06:01

FEATURE: Refactor config file support for server configuration #945

FEATURE: Refactor config file support for server configuration #945

Conversation

ing-eoking commented Dec 22, 2025

🔗 Related Issue

⌨️ What I did

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ing-eoking Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jhpark816 commented Dec 31, 2025

Uh oh!

ing-eoking commented Dec 31, 2025

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ing-eoking Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ing-eoking Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jhpark816 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jhpark816 commented Jan 7, 2026

Uh oh!

jhpark816 commented Jan 7, 2026

Uh oh!

ing-eoking Dec 29, 2025 •

edited

Loading

ing-eoking Jan 2, 2026 •

edited

Loading

ing-eoking Jan 5, 2026 •

edited

Loading