naver / arcus-memcached

ARCUS memory cache server
https://github.com/naver/arcus
Apache License 2.0
70 stars 55 forks source link

[persistence] log waiter 속성 설정과 이용 방식 변경. #450

Closed jhpark816 closed 4 years ago

jhpark816 commented 4 years ago

log waiter 속성 중에 아래 2개 필드가 있다.

이 중에, elem_insert_with_create 속성은 item 모듈의 코드와 밀겹합되어 있다. 이러한 밀결합을 제거하기 위하여, log waiter 속성을 아래와 같이 변경합시다.

결국, log waiter 생성 시의 write 연산 타입을 지정해 두고 log record 생성 시에 이용한다면, item 모듈과의 밀결합 코드를 제거할 수 있다.

jhpark816 commented 4 years ago

@MinWooJin 이슈 사항을 검토해 주세요.

MinWooJin commented 4 years ago

drop if empty는 가능합니다. create if absent는 현재 제안된 방식 자체로는 효율적이지 못한 것 같고, 또다른 flag를 하나 더 두거나 collection item 생성 작업의 log record 생성 시점에 연산의 타입을 변경해서 그 다음 생성할 log record가 attribute 정보를 포함해서 기록해야 하는지 판단 할 필요가 있습니다.

drop if empty는 flag 하나만 log record에 추가되는 방식이고, 실제 item 삭제 작업보다 먼저 element delete log record를 생성하기 때문에 drop 작업 수행이 예상될 경우엔 모두 추가한 방식이지만, create if absent는 item 생성 작업이 먼저 수행되므로 element insert log record 생성 시점에 item 생성 필요 여부를 정확하게 알 수 있어서 실제 수행 해야 할 경우에만 추가하도록 한 방식입니다.

결론은 log waiter 생성 시의 write 연산 타입을 지정해두고 log record 생성 시에 이용이 가능하고, item 모듈과의 밀결합 제거도 가능할 것 같습니다.

jhpark816 commented 4 years ago

아래와 같이 또 다른 flag가 반드시 필요한 지를 다시 한번 검토해 주세요.

create if absent는 현재 제안된 방식 자체로는 효율적이지 못한 것 같고, 또다른 flag를 하나 더 두거나 collection item 생성 작업의 log record 생성 시점에 연산의 타입을 변경해서 그 다음 생성할 log record가 attribute 정보를 포함해서 기록해야 하는지 판단 할 필요가 있습니다.

MinWooJin commented 4 years ago

실제 create 여부를 element insert log record 생성에서 알아야 하기 때문에 또 다른 flag나 create insert를 의미하는 연산 타입이 필요할 것 같습니다. 아니면 do_coll_elem_insert 수행 시 created를 나타내는 argument를 같이 넘길 수도 있습니다.

ENGINE_ERROR_CODE map_elem_insert(const char *key, const uint32_t nkey,
                                  map_elem_item *elem, item_attr *attrp,
                                  bool *created, const void *cookie)
{
...
#ifdef ENABLE_PERSISTENCE
            if (waiter != NULL) waiter->elem_insert_with_create = true;
#endif
            ret = do_item_link(it);
            if (ret == ENGINE_SUCCESS) {
                *created = true;
            } else {
                /* The item is to be released, below */
            }
        }
    }
    if (ret == ENGINE_SUCCESS) {
        ret = do_map_elem_insert(it, elem, false /* replace_if_exist */, cookie);
...
}
jhpark816 commented 4 years ago

@MinWooJin 커뮤니케이션 mistake 인 것 같습니다.

MinWooJin commented 4 years ago

@jhpark816

jhpark816 commented 4 years ago

@MinWooJin 앞 코멘트에 아래와 같이 적혀 있었네요. 아래와 같이 하면 될 것 같습니다.

collection item 생성 작업의 log record 생성 시점에 연산의 타입을 변경해서 그 다음 생성할 log record가 attribute 정보를 포함해서 기록해야 하는 지를 알게 한다.

MinWooJin commented 4 years ago

@jhpark816 네 그렇게 하면 문제 없을 것 같습니다.

jhpark816 commented 4 years ago

@MinWooJin 그럼, 진행하시죠. multiple log records의 atomic redo에서도 이러한 밀결합이 제거된 구현 방안을 검토해 주시죠.

MinWooJin commented 4 years ago

waiter operation type을 정리합니다.

enum waiter_optype {
    WOP_NONE = 0,
    /* item_store, coll_struct_create */
    WOP_ITEM_CREATE,
    /* item_arithmetic */
    WOP_ITEM_UPDATE,
    /* item_delete */
    WOP_ITEM_DELETE,
    /* coll_elem_insert */
    WOP_ELEM_INSERT,
    WOP_ELEM_CREATED_INSERT,
    /* coll_elem_update, coll_elem_arithmetic */
    WOP_ELEM_UPDATE,
    /* coll_elem_delete, coll_elem_get */
    WOP_ELEM_DELETE, 
    WOP_ELEM_DROPPED_DELETE,
    /* item_flush_expired, item_setattr */
    WOP_ADMIN
};
jhpark816 commented 4 years ago

기존에 upd_type이 있는 데, 이를 확장하여 활용하면 되지 않나 생각됩니다.

enum upd_type {
    /* key value command */
    UPD_SET = 0,
    UPD_DELETE,
    UPD_SETATTR_EXPTIME,
    UPD_SETATTR_EXPTIME_INFO,
    UPD_SETATTR_EXPTIME_INFO_BKEY,
    UPD_FLUSH,
    /* list command */
    UPD_LIST_CREATE,
    UPD_LIST_ELEM_INSERT,
    UPD_LIST_ELEM_DELETE,
    /* set command */
    UPD_SET_CREATE,
    UPD_SET_ELEM_INSERT,
    UPD_SET_ELEM_DELETE,
    /* map command */
    UPD_MAP_CREATE,
    UPD_MAP_ELEM_INSERT,
    UPD_MAP_ELEM_DELETE,
    /* btree command */
    UPD_BT_CREATE,
    UPD_BT_ELEM_INSERT,
    UPD_BT_ELEM_DELETE,
    /* not command */
    UPD_NONE
};
jhpark816 commented 4 years ago

그리고, 어떤 방안이 더 좋은 지가 검토되면 좋을 것 같습니다.

MinWooJin commented 4 years ago

@jhpark816 upd_type을 확장하는 방안도 고려해봤는데, waiter에서 사용하려는 operation type과 성격이 달라 일단은 따로 해본 상태입니다. wiater의 operation type이 실제 코드와 맞춰서 보니 조금 어색 부분들이 있어서 코멘트해주신 부분과 함께 검토 해보려 합니다.

MinWooJin commented 4 years ago

현재 issue는 PR #452 에 반영 된 상태입니다. 추가 이슈 처리가 필요 할 경우 새로운 이슈로 생성하여 처리 예정입니다. 현재 issue close 합니다.