rdsn icon indicating copy to clipboard operation
rdsn copied to clipboard

feat: support to set/get dirty_decay_ms and muzzy_decay_ms for jemalloc

Open empiredan opened this issue 3 years ago • 4 comments

In jemalloc, there are 2 important options for the two-phase, decay-based purging, with each option corresponding to a phase. They are:

And this PR will provide 2 remote commands to set/get each option dynamically even if rDSN built with jemalloc is running. The 2 commands's format are as below:

replica.set-jemalloc-arena-dirty-decay-ms <arena_index | ALL> [decay_ms | DEFAULT | DISABLE]
replica.set-jemalloc-arena-muzzy-decay-ms <arena_index | ALL> [decay_ms | DEFAULT | DISABLE]

Actually, the arguments of the 2 commands are exactly the same.

The 1st argument is required. A valid value for it should be a arena_index or ALL. Suppose that the number of arenas is n, then the range of arena_index is [0, n). ALL means setting/getting for all arenas.

The 2nd argument is optional. If it is missing, the value of dirty_decay_ms or muzzy_decay_ms will be returned, which means getting (rather than setting) a value for any arena or the values for all arenas.

If the 2nd argument is given, a valid value for it should be a decay_ms, DEFAULT, or DISABLE. Just as its name implies, decay_ms means the numerical value of dirty_decay_ms or muzzy_decay_ms, whose unit is milliseconds. The value is valid only if it's equal or more than 0. Or, it should be either of the following values:

  • DEFAULT: for dirty_decay_ms, the value is 10000, namely 10 seconds; for muzzy_decay_ms, the value is 0, which means immediately purging once dirty/muzzy pages are created.
  • DISABLE: for both dirty_decay_ms and muzzy_decay_ms, the value is -1, which means disabling purging.

empiredan avatar Sep 29 '21 13:09 empiredan

Is it possible to add some function/unit tests? It's a big PR, and I am not sure its correctness

Smityz avatar Oct 25 '21 03:10 Smityz

Is it possible to add some function/unit tests? It's a big PR, and I am not sure its correctness

@Smityz Yes, actually as is discussed with @acelyc111, I also worry about this problem, even jemalloc related code will not be compiled for each commit since now the macro USE_JEMALLOC is disabled. I'll add some unit tests.

empiredan avatar Oct 25 '21 03:10 empiredan

Is it possible to add some function/unit tests? It's a big PR, and I am not sure its correctness

@Smityz Yes, actually as is discussed with @acelyc111, I also worry about this problem, even jemalloc related code will not be compiled for each commit since now the macro USE_JEMALLOC is disabled. I'll add some unit tests.

I think a better way to check jemalloc is to do smoke test, for example, add a jemalloc test section in .github/workflows/pull_request.yaml, i.e. enable USE_JEMALLOC, and run all unit tests.

acelyc111 avatar Nov 27 '21 04:11 acelyc111

Is it possible to add some function/unit tests? It's a big PR, and I am not sure its correctness

@Smityz Yes, actually as is discussed with @acelyc111, I also worry about this problem, even jemalloc related code will not be compiled for each commit since now the macro USE_JEMALLOC is disabled. I'll add some unit tests.

I think a better way to check jemalloc is to do smoke test, for example, add a jemalloc test section in .github/workflows/pull_request.yaml, i.e. enable USE_JEMALLOC, and run all unit tests.

Yes, we can add a new workflow

Smityz avatar Dec 16 '21 06:12 Smityz