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

FEATURE: touch command #778

Closed
wants to merge 8 commits into from
Closed

Conversation

cheesecrust
Copy link

@cheesecrust cheesecrust commented Jul 5, 2024

Description

A feature to alter the expiration time of a key without fetching the item.
This works for KV relationships and collections.

Syntax

touch <key> <exptime> [noreply]\r\n

Response

  • "TOUCHED\r\n" to indicate success
  • "NOT_FOUND\r\n" to indicate that the item with this key was not
    found.

@namsic namsic requested review from namsic and ing-eoking July 5, 2024 09:01
Copy link
Collaborator

@namsic namsic left a comment

Choose a reason for hiding this comment

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

전반적으로 코드 정리가 필요해 보입니다.
코드 포맷이나 불필요한 코드는 없는지 다시 확인해 주세요.

Dockerfile Outdated Show resolved Hide resolved
memcached.c Outdated Show resolved Hide resolved
@cheesecrust cheesecrust changed the title Arcus-memcached touch FEATURE: arcus-memcached touch Jul 8, 2024
@cheesecrust cheesecrust changed the title FEATURE: arcus-memcached touch FEATURE: touch command Jul 8, 2024
memcached.c Outdated Show resolved Hide resolved
memcached.c Outdated Show resolved Hide resolved
memcached.c Outdated Show resolved Hide resolved
memcached.c Outdated Show resolved Hide resolved
@ing-eoking

This comment was marked as resolved.

@cheesecrust cheesecrust marked this pull request as draft July 8, 2024 02:04
@cheesecrust cheesecrust marked this pull request as ready for review July 8, 2024 03:13
memcached.c Outdated Show resolved Hide resolved
memcached.c Outdated Show resolved Hide resolved
memcached.c Show resolved Hide resolved
@cheesecrust cheesecrust marked this pull request as draft July 9, 2024 00:15
memcached.c Show resolved Hide resolved
@cheesecrust cheesecrust force-pushed the touch branch 2 times, most recently from 4a5406b to 82d1237 Compare July 9, 2024 01:36
@ing-eoking ing-eoking marked this pull request as ready for review July 9, 2024 02:40
@ing-eoking ing-eoking requested a review from namsic July 9, 2024 02:40
Copy link
Collaborator

@namsic namsic left a comment

Choose a reason for hiding this comment

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

코드 정리에 더 신경써 주기 바랍니다.

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

@cheesecrust
아래 추가 바랍니다.

  • DOCS에 명령 설명 추가
  • make test 시에 unit testing할 수 있도록 간단한 test 추가.

@cheesecrust cheesecrust marked this pull request as draft July 9, 2024 07:24
@cheesecrust cheesecrust marked this pull request as ready for review July 9, 2024 08:54
@ing-eoking
Copy link
Collaborator

mem_cmd_is 에 대한 참고입니다.

sub mem_cmd_is {
my ($sock_opts, $cmd, $val, $rst, $msg) = @_;
my @response_list;
my @response_error = ("ATTR_ERROR", "CLIENT_ERROR", "ERROR", "PREFIX_ERROR", "SERVER_ERROR");
my @prdct_response = split('\n', $rst);
my @cmd_pipeline = split('\r\n', $cmd);
my $rst_type = 0;
my $count;
my $opts = ref $sock_opts eq "HASH" ? $sock_opts : {};
my $sock = ref $sock_opts eq "HASH" ? $opts->{sock} : $sock_opts;
# send command
if ("$val" eq "") {
print $sock "$cmd\r\n";
} else {
print $sock "$cmd\r\n$val\r\n";
}
# msg string
if ("$msg" eq "") {
if (length($val) <= 40) {
$msg = $cmd . ":" . $val;
} else {
$msg = $cmd . ":" . substr($val, 0, 40);
}
}
my $resp = "";
my $line;
if ($cmd =~ /noreply$/) {
if ("$rst" ne "") {
$rst_type = 3;
}
} elsif ($#cmd_pipeline > 1) {
$rst_type = 3;
} else {
$line = scalar <$sock>;
$resp = $resp . (substr $line, 0, length($line)-2);
if ($line =~ /^VALUE/) {
$rst_type = 1;
@response_list = ("DELETED", "DELETED_DROPPED", "DUPLICATED", "DUPLICATED_TRIMMED", "END", "TRIMMED");
} elsif ($line =~ /^ELEMENTS/) {
$rst_type = 1;
@response_list = ("DUPLICATED", "END");
} elsif (grep $resp =~ /^$_/, @response_error) {
$rst_type = 2;
} elsif ($line =~ /^RESPONSE/) { # pipe command
$rst_type = 2;
} elsif ($line =~ /^(ATTR|PREFIX)/) {
$rst_type = 1;
@response_list = ("END");
} else {
# single line response
# @response_list = ("ATTR_MISMATCH", "BKEY_MISMATCH", "CREATED", "CREATED_STORED"
# , "DELETED", "DELETED_DROPPED", "DUPLICATED", "DUPLICATED_TRIMMED"
# , "EFLAG_MISMATCH", "ELEMENT_EXISTS", "END", "EXIST", "EXISTS", "NOT_EXIST", "NOT_FOUND"
# , "NOT_FOUND_ELEMENT", "NOT_STORED", "NOT_SUPPORTED", "NOTHING_TO_UPDATE", "OK"
# , "OUT_OF_RANGE", "OVERFLOWED", "REPLACED", "RESET", "STORED", "TRIMMED"
# , "TYPE_MISMATCH", "UNREADABLE", "UPDATED"
# , "ATTR_ERROR", "CLIENT_ERROR", "ERROR", "PREFIX_ERROR", "SERVER_ERROR"
# , "COUNT=", "POSITION=");
}
}
if ($rst_type eq 1) {
do {
$resp = $resp . "\n";
$line = scalar <$sock>;
$resp = $resp . (substr $line, 0, length($line)-2);
} while (!(grep $line =~ /^$_/, @response_list));
} elsif ($rst_type eq 2) {
$count = $#prdct_response;
while ($count--) {
$resp = $resp . "\n";
$line = scalar <$sock>;
$resp = $resp . (substr $line, 0, length($line)-2);
}
} elsif ($rst_type eq 3) {
$count = $#prdct_response + 1;
while ($count--) {
$line = scalar <$sock>;
$resp = $resp . (substr $line, 0, length($line)-2);
if ($count eq 0) {
last;
}
$resp = $resp . "\n";
}
}
Test::More::is("$resp", "$rst", $msg);
}

t/touch.t Outdated Show resolved Hide resolved
t/touch.t Outdated Show resolved Hide resolved
t/touch.t Show resolved Hide resolved
docs/ascii-protocol/ch10-command-item-attribute.md Outdated Show resolved Hide resolved
@cheesecrust cheesecrust force-pushed the touch branch 2 times, most recently from 41622c7 to 0e3c76e Compare July 10, 2024 03:35
@cheesecrust cheesecrust force-pushed the touch branch 2 times, most recently from 87ce740 to 6bdc30b Compare July 10, 2024 04:19
t/touch.t Outdated Show resolved Hide resolved
t/touch.t Outdated Show resolved Hide resolved
@cheesecrust cheesecrust force-pushed the touch branch 2 times, most recently from ade70c2 to 0d22499 Compare July 11, 2024 06:59
|-----------------------------------------|------------------------ |
| "TOUCHED" | 성공
| "NOT_FOUND" | key miss
| "CLIENT_ERROR bad value" | exptime 값이 유효하지 않은 경우
Copy link
Collaborator

Choose a reason for hiding this comment

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

@namsic
touch 명령은 memcached의 기존 kv 명령이므로
이 용을 kv 부분으로 옮기는 것이 좋을 것 같습니다.
검토해 주세요.

Copy link
Author

Choose a reason for hiding this comment

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

해당 부분을 ch10 에서 kv 부분인 ch04 로 옮겼습니다.

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 의견이 조금 다른데, touch는 attribute에, gat는 kv에 두는 것이 좋아 보입니다.

  1. memcached에서는 <cache item> == <kv item>이므로 touch를 kv 명령으로 볼 수 있지만,
  2. arcus에서는 touch 명령으로 collection item의 exptime도 설정할 수 있기 때문입니다.
  3. @cheesecrust 만약 touch 문서를 kv에 두려면, collection type의 문서에서도 관련 내용이 언급되어야 맞는 것 같습니다.

add touch command with test file and docs
@ing-eoking ing-eoking marked this pull request as draft August 2, 2024 02:17
@cheesecrust cheesecrust marked this pull request as ready for review August 23, 2024 07:25
@cheesecrust cheesecrust marked this pull request as draft August 23, 2024 07:25
@cheesecrust
Copy link
Author

cheesecrust commented Aug 23, 2024

#782
커밋을 하나로 병합해 위 새 pr로 올렸습니다.
이에 closed 합니다.

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.

5 participants