lock_arcus 사용 용도 검토 및 최적화 #236

jhpark816 · 2022-02-22T12:21:28Z

arcus.cc 파일에 있는 lock_arcus locking 관련하여 아래 작업을 진행합시다.

lock_arcus locking 현재 용도 파악
불필요한 lock_arcus locking 확인
추가해야 할 lock_arcus locking 확인

본 이슈에 결과를 정리해 두면 됩니다.
결과에 따라 lock_arcus locking 제거 or 추가를 진행하도록 하시죠.

uhm0311 · 2022-02-25T09:20:22Z

@jhpark816

lock_arcus locking 현재 용도 파악

mc->server_manager 객체를 보호하기 위함인 것으로 보입니다. 해당 객체는 ZK manager thread와 worker thread 양쪽에서 모두 사용합니다.

불필요한 lock_arcus locking 확인

불필요한 부분은 없는 것으로 보입니다.

추가해야 할 lock_arcus locking 확인

arcus_pool_connect() 함수와 같이 mc->server_manager 객체를 사용하면서 lock을 하지 않는 곳에 추가하면 될 것 같습니다.
추가적으로, 아래의 예시와 같이 locking이 일관적이지 않은 부분이 있습니다.

arcus-c-client/libmemcached/arcus.cc

Lines 1117 to 1123 in cfe538c

    
           pthread_mutex_lock(&lock_arcus); 
        
           arcus_st *arcus= static_cast<arcus_st *>(memcached_get_server_manager(mc)); 
        
           pthread_mutex_unlock(&lock_arcus); 
        
           if (not arcus || not arcus->zk.handle) { 
        
             ZOO_LOG_ERROR(("arcus is null")); 
        
             return; 
        
           }

위는 do_arcus_zk_watcher_cachelist() 함수의 일부분입니다.

arcus-c-client/libmemcached/arcus.cc

Lines 1150 to 1156 in cfe538c

    
           pthread_mutex_lock(&lock_arcus); 
        
           arcus= static_cast<arcus_st *>(memcached_get_server_manager(mc)); 
        
           if (not arcus || not arcus->zk.handle) { 
        
             ZOO_LOG_ERROR(("arcus is null")); 
        
             pthread_mutex_unlock(&lock_arcus); 
        
             return; 
        
           }

위는 do_arcus_zk_watcher_global() 함수의 일부분입니다.
두 코드는 lock을 제외하면 동일한 동작을 수행하는 것인데, unlock 시점이 서로 다릅니다. 어느 한 쪽으로 통일하면 좋을 것 같습니다.

jhpark816 · 2022-02-25T10:14:43Z

@uhm0311

아래 작업을 하는 데, locking 필요한가요?
이유는 무엇인가요?

 pthread_mutex_lock(&lock_arcus); 
 arcus_st *arcus= static_cast<arcus_st *>(memcached_get_server_manager(mc)); 
 pthread_mutex_unlock(&lock_arcus);

uhm0311 · 2022-02-25T10:20:31Z

@jhpark816

memcached_set_server_manager() 함수를 수행 할 때 lock을 하고 있습니다. 그래서 memcached_get_server_manager() 함수를 수행할 때도 필요하다고 생각했습니다.

jhpark816 · 2022-02-25T10:45:52Z

@uhm0311
set과 get 수행 시점의 상황을 확인하여 locking 필요한 지 검토하기 바랍니다.
예를 들어,

get 요청이 들어오지 않는 상황에서 처음 1회 set 요청하고,
그 이후에 get 요청만 들어온다면 locking은 필요 없습니다.
이러한 경우에 해당되는 지를 확인하기 바랍니다.

uhm0311 · 2022-03-02T07:19:51Z

@jhpark816

arcus 객체에는 크게 네 종류의 thread에서 접근합니다.
- creation thread (memcached_create() 함수를 호출하는 thread)
- worker thread
- zk manager thread
- zk watcher thread (do_arcus_zk_watcher_...() 함수를 호출하는 thread)
creation thread에서의 접근은 다른 두 thread가 활성화 되기 전에 malloc하고 다른 두 thread가 비활성 된 뒤에 free 합니다.
worker thread에 의한 접근은 arcus_server_check_for_update() 함수에서 이뤄집니다.
zk manager thread에 의한 접근은 do_arcus_zk_process_reconnect() 함수, do_arcus_zk_watch_and_update_cachelist() 함수에서 이뤄집니다.
zk watcher thread에 의한 접근은 do_arcus_zk_watcher_global() 함수, do_arcus_zk_watcher_cachelist() 함수에서 이뤄집니다.
만약 zk manager thread와 zk watcher thread가 같은 thread라면 lock_arcus가 불필요해 보입니다.
- 서로 다른 thread라면 두 thread 사이에서만 lock을 잡아주면 될 것 같습니다.
여러 worker thread에 의해 cachelist update가 이뤄질 수 있으므로 arcus_server_check_for_update() 함수에도 적절한 lock이 이뤄져야 하지만 그 lock이 꼭 lock_arcus일 필요는 없어 보입니다.

jhpark816 · 2022-03-02T07:25:49Z

@uhm0311
zk watcher thread는 zk manager thread와 다른 thread입니다.

ZK client 내부에서 생성되어 watcher logic 수행하는 thread입니다.

어떤 부분에서 어떤 locking 필요한 지 정리해 보면 될 것 같습니다.

uhm0311 · 2022-03-02T07:34:05Z

@jhpark816

zk manager thread와 zk watcher thread 사이에서 arcus->zk.myid가 보호되어야 합니다.
- do_arcus_zk_process_reconnect() 함수에서 arcus->zk.myid를 read, write 합니다.
- do_arcus_zk_watcher_global() 함수에서 arcus->zk.myid를 read, write 합니다.
이 외에는 없어 보입니다. arcus->zk.myid에 접근하는 코드만 lock을 걸어주면 될 것 같습니다.
lock_arcus 대신 다른 이름의 lock을 사용하면 될 것 같습니다.
- arcus->zk_mgr.lock처럼, arcus->zk.lock을 만들어서 사용하면 어떨까요?

jhpark816 · 2022-03-02T08:12:22Z

@uhm0311
코드를 보지 않았지만,
zk.myid read/write 외에 locking 필요 없다는 부분을 다시 검증해 보시죠.

uhm0311 · 2022-03-02T09:30:40Z

@jhpark816

arcus->zk.conn_result가 creation thread에 의해 read, zk watcher thread에 의해 write가 일어나는 부분이 있습니다. 그런데 read를 하기 전 wait_count() 함수를 이용해 동시 접근 제어를 하고 있습니다. read 하기 전 wait 상태에 들어간 뒤 write가 일어난 후에 inc_count(-1)로 wait 상태에서 빠져 나와 read 합니다.
이 외에는 서로 다른 thread에 의해 동시에 read/write가 일어나는 부분이 없어 보입니다.

jhpark816 · 2022-03-03T02:07:04Z

@uhm0311

arcus_lock 이름은 그대로 사용합니다.
PR 보내주면 되나요?

uhm0311 · 2022-03-03T02:09:40Z

@jhpark816

네 그러면 될 것 같습니다.

jhpark816 · 2022-03-10T09:36:23Z

@uhm0311
PR 보내주면, 코드 보고 판단할게요.

jhpark816 · 2022-03-14T11:22:55Z

@uhm0311
PR 코드를 봐도, 이렇게 수정하는 것이 맞는 지 검증이 되지 않습니다.
본 이슈에 lock_arcus 필요성이 여러 코멘트에 흩어져 있어서, 정리가 잘 되지 않습니다.
lock_arcus 목적과 locking 필요한 부분을 하나의 코멘트에서 요약하여 정리해 보시죠.

uhm0311 · 2022-03-15T04:40:29Z

@jhpark816

lock_arcus는 mc->server_manager, 즉 arcus 객체를 보호하기 위해 사용됩니다.
- arcus 객체에는 크게 네 종류의 thread에서 접근합니다.
  - creation thread (memcached_create() 함수를 호출하는 thread)
  - worker thread
  - zk manager thread
  - zk watcher thread (do_arcus_zk_watcher_...() 함수를 호출하는 thread)
lock_arcus의 필요성
- creation thread에서의 접근은 다른 세 thread가 활성화 되기 전에 malloc하고 다른 세 thread가 비활성 된 뒤에 free 합니다.
  - 다른 thread가 접근할 수 없는 경우에 한해서만 write가 이뤄지므로 creation thread에 의한 접근은 lock할 필요가 없습니다.
- worker thread에 의한 접근은 arcus_server_check_for_update() 함수에서 이뤄집니다.
  - 여러 worker thread에 의해 serverlist update가 이뤄질 수 있으므로 arcus_server_check_for_update() 함수에도 적절한 lock이 이뤄져야 하지만 그 lock이 꼭 lock_arcus일 필요는 없습니다.
- zk manager thread에 의한 접근은 do_arcus_zk_process_reconnect() 함수, do_arcus_zk_watch_and_update_cachelist() 함수에서 이뤄집니다. zk watcher thread에 의한 접근은 do_arcus_zk_watcher_global() 함수, do_arcus_zk_watcher_cachelist() 함수에서 이뤄집니다.
- creation thread를 제외하고 서로 다른 thread에 의해 동시에 read/write 하는 자원에 대해서만 lock을 잡으면 됩니다.
- 해당하는 자원은 arcus->proxy 객체, arcus->zk_mgr 객체, arcus->zk.conn_result 객체, arcus->zk.myid 객체입니다.
  - arcus->proxy 객체는 arcus->proxy.data->mutex로 lock을 잡고 있습니다.
  - arcus->zk_mgr 객체는 arcus->zk_mgr.lock으로 lock을 잡고 있습니다.
  - arcus->zk.conn_result 객체는 read 하기 전 wait_count()로 lock을 잡고 write 한 후 inc_count(-1)로 lock을 풉니다.
  - arcus->zk.myid 객체는 큰 범위의 lock_arcus로 lock을 잡고 있습니다. 그러나 arcus->zk.myid 객체 외에 lock_arcus로 arcus 객체를 보호할 필요가 없다고 판단하여 다른 부분의 lock_arcus 사용은 제거하고 arcus->zk.myid 객체 접근 시에만 lock_arcus로 lock을 잡고자 합니다.

jhpark816 · 2022-03-15T06:12:53Z

@uhm0311
thread-safe API 제공하기 위한 의도는 없다고 보나요?
즉, 응용에서 여러 thread가 같은 API를 호출할 경우에 대한 대처 목적은 없다고 보나요?

uhm0311 · 2022-03-15T06:27:14Z

@jhpark816

응용에서 접근할 수 있는 arcus.cc 파일의 API 목록은 다음과 같습니다.
1. arcus_connect();
2. arcus_close();
3. arcus_pool_connect();
4. arcus_pool_close();
5. arcus_proxy_create();
6. arcus_proxy_connect();
7. arcus_proxy_close();
8. arcus_strerror();
9. arcus_set_log_stream();
10. arcus_server_check_for_update();
arcus_strerror() 함수와 arcus_set_log_stream() 함수는 arcus 객체를 사용하지 않습니다.
arcus_server_check_for_update() 함수에서 serverlist update 수행 시 적절한 lock이 필요하지만 반드시 lock_arcus를 사용할 필요는 없습니다. lock_update_serverlist라는 새로운 mutex로 lock을 잡았습니다.
arcus_strerror(), arcus_set_log_stream(), arcus_server_check_for_update() 세 함수를 제외하면 creation thread에서 1회 호출하는 함수입니다.

응용에서 여러 thread가 같은 API를 호출할 경우에 대한 대처 목적은 없다고 보나요?

arcus_server_check_for_update() 함수가 이에 해당됩니다. proxy를 사용하는 경우 arcus->proxy.data->mutex로 lock을 잡습니다. 그렇지 않은 경우 memcached_pool_update_member() 함수를 호출하는데, 기존에 lock_arcus로 lock을 잡았으나 lock_update_serverlist라는 새로운 mutex로 대체했습니다.

jhpark816 assigned SuhwanJang and uhm0311 and unassigned SuhwanJang Feb 22, 2022

jhpark816 added the ongoing label Feb 22, 2022

naver deleted a comment from uhm0311 Mar 2, 2022

uhm0311 added a commit to uhm0311/arcus-c-client that referenced this issue Mar 14, 2022

INTERNAL: Remove lock_arcus that are not in necessary naver#236

3b30b8e

uhm0311 added a commit to uhm0311/arcus-c-client that referenced this issue Mar 14, 2022

INTERNAL: Remove lock_arcus that are not in necessary naver#236

4625554

uhm0311 added a commit to uhm0311/arcus-c-client that referenced this issue Mar 15, 2022

INTERNAL: Remove lock_arcus that are not in necessary naver#236

2a4d2fe

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lock_arcus 사용 용도 검토 및 최적화 #236

lock_arcus 사용 용도 검토 및 최적화 #236

jhpark816 commented Feb 22, 2022

uhm0311 commented Feb 25, 2022

jhpark816 commented Feb 25, 2022 •

edited

Loading

uhm0311 commented Feb 25, 2022

jhpark816 commented Feb 25, 2022

uhm0311 commented Mar 2, 2022

jhpark816 commented Mar 2, 2022

uhm0311 commented Mar 2, 2022 •

edited

Loading

jhpark816 commented Mar 2, 2022

uhm0311 commented Mar 2, 2022

jhpark816 commented Mar 3, 2022

uhm0311 commented Mar 3, 2022

jhpark816 commented Mar 10, 2022

jhpark816 commented Mar 14, 2022

uhm0311 commented Mar 15, 2022

jhpark816 commented Mar 15, 2022 •

edited

Loading

uhm0311 commented Mar 15, 2022

lock_arcus 사용 용도 검토 및 최적화 #236

lock_arcus 사용 용도 검토 및 최적화 #236

Comments

jhpark816 commented Feb 22, 2022

uhm0311 commented Feb 25, 2022

jhpark816 commented Feb 25, 2022 • edited Loading

uhm0311 commented Feb 25, 2022

jhpark816 commented Feb 25, 2022

uhm0311 commented Mar 2, 2022

jhpark816 commented Mar 2, 2022

uhm0311 commented Mar 2, 2022 • edited Loading

jhpark816 commented Mar 2, 2022

uhm0311 commented Mar 2, 2022

jhpark816 commented Mar 3, 2022

uhm0311 commented Mar 3, 2022

jhpark816 commented Mar 10, 2022

jhpark816 commented Mar 14, 2022

uhm0311 commented Mar 15, 2022

jhpark816 commented Mar 15, 2022 • edited Loading

uhm0311 commented Mar 15, 2022

jhpark816 commented Feb 25, 2022 •

edited

Loading

uhm0311 commented Mar 2, 2022 •

edited

Loading

jhpark816 commented Mar 15, 2022 •

edited

Loading