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

INTERNAL: Move a default_topkeys variable #783

Closed
wants to merge 1 commit into from

Conversation

ing-eoking
Copy link
Collaborator

@ing-eoking ing-eoking commented Sep 11, 2024

πŸ”— Related Issue

  • jam2in/arcus-works#506

⌨️ What I did

  • default_topkeys λ³€μˆ˜λ₯Ό memcached.c μ—μ„œ topkey.c 둜 μ΄λ™μ‹œν‚΅λ‹ˆλ‹€.
  • memcached.hμ—μ„œ μ „μ—­ν™”λ˜μ–΄ μžˆλŠ” λ³€μˆ˜μ΄κΈ°μ— ν”„λ‘œν† μ½œ 뢄리에 λ¬Έμ œλŠ” μ—†μœΌλ‚˜ topkey에 κ΄€ν•œ λ³€μˆ˜μ΄λ―€λ‘œ topkey λͺ¨λ“ˆμ— μžˆλŠ” 편이 μžμ—°μŠ€λŸ½λ‹€κ³  μƒκ°ν–ˆμŠ΅λ‹ˆλ‹€.

@jhpark816 jhpark816 removed their request for review September 11, 2024 07:06
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.

lqdetect와 cmdlog의 κ²½μš°μ— in_use boolean λ³€μˆ˜λ₯Ό externν•˜κ³ ,
memcachedμ—μ„œλŠ” κΈ°λŠ₯의 ν™œμ„±ν™” μ—¬λΆ€λ₯Ό ν™•μΈν•˜κ³  ν•„μš”ν•œ ν•¨μˆ˜λ₯Ό ν˜ΈμΆœν•˜κ²Œ λ©λ‹ˆλ‹€.

κ°„λ‹¨νžˆ λ³΄μ•˜μ„ λ•Œ memcached.c μͺ½μ—μ„œλŠ” default_topkeysλ₯Ό xxxxxx_in_use와 거의 μœ μ‚¬ν•œ λͺ©μ μœΌλ‘œ μ‚¬μš©ν•˜λŠ” 것 같은데,
topkeys도 lqdetect λ˜λŠ” cmdlog와 μœ μ‚¬ν•œ ν˜•νƒœλ‘œ λ³€κ²½ν•˜λŠ” 것은 어떀지 의견 μ£Όμ‹œλ©΄ μ’‹κ² μŠ΅λ‹ˆλ‹€.
(λ³€κ²½ν–ˆμ„ λ•Œ λ¬Έμ œκ°€ 될 λ§Œν•œ 뢀뢄이 μžˆλŠ”μ§€, κ°œλ… 상 μ μ ˆν•˜μ§€ μ•Šμ€ ν˜•νƒœμΈμ§€ λ“±)

  • default_topkeysλŠ” topkeys.c의 static λ³€μˆ˜λ‘œ λ³€κ²½
  • 기쑴에 default_topkeysλ₯Ό 인자둜 λ°›λ˜ ν•¨μˆ˜λŠ” κΈ€λ‘œλ²Œ λ³€μˆ˜ default_topkeysλ₯Ό μ‚¬μš©ν•˜λ„λ‘ λ³€κ²½
  • default_topkeys λŒ€μ‹  boolean λ³€μˆ˜λ₯Ό extern
// AS-IS
if (default_topkeys) {
    topkeys_stats(default_topkeys, c, get_current_time(), append_bin_stats);
}

// TO-BE
if (topkey_in_use) {
    topkeys_stats(c, get_current_time(), append_bin_stats);
}

@jhpark816
Copy link
Collaborator

zookeeper μ‚¬μš©ν•˜λŠ” μ‘μš©μ€ μ•„λž˜μ™€ 같이 ν˜ΈμΆœν•˜μ—¬ zk handle을 λ¦¬ν„΄λ°›μŠ΅λ‹ˆλ‹€.
그리고, zk handle을 가지고 zookeeper 연산을 ν˜ΈμΆœν•©λ‹ˆλ‹€.

    zinfo->zh = zookeeper_init(zinfo->ensemble_list, arcus_zk_watcher,
                               arcus_conf.zk_timeout, &zinfo->myid, zinfo, 0);

default_topkeys도 λ§ˆμ°¬κ°€μ§€λ‘œ topkeys κΈ°λŠ₯ μ‚¬μš©μ— λŒ€ν•œ handle 같은 κ°œλ…μž…λ‹ˆλ‹€.
λ”°λΌμ„œ, μ΄λŠ” topkeys κΈ°λŠ₯을 μ‚¬μš©ν•˜λŠ” μ‘μš©μΈ memcached.c νŒŒμΌμ— μžˆλŠ” 것이 μ •μƒμž…λ‹ˆλ‹€.

    if (settings.topkeys > 0) {
        default_topkeys = topkeys_init(settings.topkeys);
    }

@ing-eoking
Copy link
Collaborator Author

@jhpark816

ν˜„μž¬λŠ” default_topkeys만 μ‚¬μš© 쀑인데, κ·ΈλŸΌμ—λ„ topkeys_init ν•¨μˆ˜κ°€ μ—¬λŸ¬ topkeys handlerλ₯Ό λ°˜ν™˜ν•˜λ„λ‘ κ΅¬ν˜„λœ 것은, λ‹€μ–‘ν•œ ν•Έλ“€λŸ¬λ₯Ό μ‚¬μš©ν•  수 μžˆλŠ” 상황을 κ³ λ €ν•œ κ²ƒμΈκ°€μš”?

@jhpark816
Copy link
Collaborator

topkeys κΈ°λŠ₯이 λ‹€μ–‘ν•œ ν•Έλ“€(handle)을 κ°€μ§ˆ 수 μžˆλ„λ‘ κ΅¬ν˜„λœ 것인지λ₯Ό ν™•μΈν•˜μ§€ μ•Šμ€ μƒνƒœμ΄λ―€λ‘œ, ν•œλ²ˆ 봐 μ£Όμ„Έμš”.
μ—¬νŠΌ, topkeys κΈ°λŠ₯의 ν˜„μž¬ μ‚¬μš©λ²•μœΌλ‘œ 보면, 핸듀을 μ‘μš©μ—μ„œ 가지고 μžˆλŠ” 것이 λ§žλ‹€λΌλŠ” κ²ƒμž…λ‹ˆλ‹€.

@ing-eoking
Copy link
Collaborator Author

μ—¬νŠΌ, topkeys κΈ°λŠ₯의 ν˜„μž¬ μ‚¬μš©λ²•μœΌλ‘œ 보면, 핸듀을 μ‘μš©μ—μ„œ 가지고 μžˆλŠ” 것이 λ§žλ‹€λΌλŠ” κ²ƒμž…λ‹ˆλ‹€.

κΈ°μ‘΄ 둜직이 λ§žλŠ” λ°©ν–₯μ΄λ―€λ‘œ ν˜„μž¬ PR은 λ‹«κ² μŠ΅λ‹ˆλ‹€.

@ing-eoking ing-eoking closed this Sep 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