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

FIX: Modify mismatched error types when get_item_info failed #739

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

ing-eoking
Copy link
Collaborator

  • jam2in/arcus-works#510

get_item_info 실패 시 설정 에러를 ENOMEM으로 통일합니다.

@ing-eoking ing-eoking changed the title Err FIX: Modify mismatched error types when get_item_info failed Mar 5, 2024
@ing-eoking ing-eoking marked this pull request as draft March 5, 2024 04:23
@ing-eoking ing-eoking closed this Mar 5, 2024
@ing-eoking

This comment was marked as outdated.

@ing-eoking

This comment was marked as outdated.

memcached.c Outdated Show resolved Hide resolved
memcached.c Show resolved Hide resolved
Copy link
Collaborator

@jhpark816 jhpark816 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

리뷰 완료

if (hinfo_check_ascii_tail_string(&c->hinfo) != 0) { /* check "\r\n" */
out_string(c, "CLIENT_ERROR bad data chunk");
ret = ENGINE_ENOMEM;
} else if (hinfo_check_ascii_tail_string(&c->hinfo) != 0) { /* check "\r\n" */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아래 형태가 좋을 것 같은 데, 어떤가요?

  • 앞의 if 에서는 어떤 오류인지를 알고 있어 out_string 바로 호출하고,
    마지막 else 문에서만 switch 사용하여 오류에 따라 메세지를 선택한다.
  • ENOMEM도 그 의미가 다를 수 있어, 메세지를 다르게 할 수 있다.
    • get_item_info() 오류의 메세지
    • store()에서 slab memory 부족시의 메세지
    item *it = c->item;
    ENGINE_ERROR_CODE ret;
    if (!mc_engine.v1->get_item_info(mc_engine.v0, c, it, &c->hinfo)) {
        mc_logger->log(EXTENSION_LOG_WARNING, c,
                       "%d: Failed to get item info\n", c->sfd);
        out_string(c, "SERVER_ERROR out of memory for getting item info")
        ret = ENGINE_ENOMEM;
    } else if (hinfo_check_ascii_tail_string(&c->hinfo) != 0) { /* check "\r\n" */
        out_string(c, "CLIENT_ERROR bad data chunk");
        ret = ENGINE_EBADVALUE;
    } else {
        . . .
    }

    if (c->store_op == OPERATION_CAS) {
        update_stat_cas(c, ret);
    } else {
        STATS_CMD(c, set, c->hinfo.key, c->hinfo.nkey);
    }
    mc_engine.v1->release(mc_engine.v0, c, c->item);
    c->item = 0;

This comment was marked as resolved.

@@ -3709,7 +3706,7 @@ static void complete_update_bin(conn *c)
mc_engine.v1->release(mc_engine.v0, c, it);
mc_logger->log(EXTENSION_LOG_WARNING, c,
"%d: Failed to get item info\n", c->sfd);
write_bin_packet(c, PROTOCOL_BINARY_RESPONSE_EINTERNAL, 0);
write_bin_packet(c, PROTOCOL_BINARY_RESPONSE_ENOMEM, 0);

This comment was marked as outdated.

This comment was marked as off-topic.

memcached.c Outdated
@@ -7107,7 +7104,7 @@ static void process_bin_update(conn *c)
if (ret == ENGINE_SUCCESS && !mc_engine.v1->get_item_info(mc_engine.v0,
c, it, &c->hinfo)) {
mc_engine.v1->release(mc_engine.v0, c, it);
write_bin_packet(c, PROTOCOL_BINARY_RESPONSE_EINTERNAL, 0);
write_bin_packet(c, PROTOCOL_BINARY_RESPONSE_ENOMEM, 0);

This comment was marked as outdated.

@@ -8474,8 +8471,7 @@ static void process_update_command(conn *c, token_t *tokens, const size_t ntoken
if (ret == ENGINE_SUCCESS) {
if (!mc_engine.v1->get_item_info(mc_engine.v0, c, it, &c->hinfo)) {
mc_engine.v1->release(mc_engine.v0, c, it);
out_string(c, "SERVER_ERROR error getting item data");
ret = ENGINE_FAILED; /* FIXME: error type */
ret = ENGINE_ENOMEM;

This comment was marked as outdated.

This comment was marked as outdated.

jhpark816

This comment was marked as resolved.

jhpark816

This comment was marked as resolved.

@ing-eoking ing-eoking marked this pull request as draft March 8, 2024 00:28
@ing-eoking ing-eoking marked this pull request as ready for review March 8, 2024 00:42
@ing-eoking ing-eoking marked this pull request as draft March 8, 2024 00:53
@ing-eoking ing-eoking marked this pull request as ready for review March 8, 2024 02:57
@ing-eoking
Copy link
Collaborator Author

수정되었습니다.

memcached.c Outdated Show resolved Hide resolved
FIX: Modify mismatched error types when get_item_info failed

Co-authored-by: Namjae Kim <kimnj3050@gmail.com>
@jhpark816 jhpark816 merged commit dbafb3c into naver:develop Mar 11, 2024
1 check passed
@ing-eoking ing-eoking deleted the err branch March 11, 2024 06:07
@ing-eoking ing-eoking self-assigned this May 13, 2024
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

Successfully merging this pull request may close these issues.

3 participants