You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "aleksraiden (via GitHub)" <gi...@apache.org> on 2023/06/24 09:43:09 UTC
[GitHub] [kvrocks] aleksraiden opened a new pull request, #1516: (deps) Bump rocksdb v8.3.2
aleksraiden opened a new pull request, #1516:
URL: https://github.com/apache/kvrocks/pull/1516
Bump Rocksdb v8.3.2
In this release:
- (perf) Improved the I/O efficiency of prefetching SST metadata by recording more information in the DB manifest
- (perf) CPU features are no longer detected at runtime nor in build scripts, but in source code using common preprocessor defines
- (bug) Delete an empty WAL file on DB open
- (bug) Delete temp OPTIONS file on DB open
- (bug) Fixed higher read QPS during DB::Open()
- (bug) Reduced cases of illegally using Env::Default()
- (feat) New statistics tickers
- (feat) Add new API DB::ClipColumnFamily to clip the key in CF to a certain range. It will physically deletes all keys outside the range including tombstones.
- (feat) Add MakeSharedCache() construction functions to various cache Options objects
- (feat) Changed the meaning of various Bloom filter stats
Full release doc: https://github.com/facebook/rocksdb/releases/tag/v8.3.2
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#issuecomment-1605442357
In this version of rocksdb changes a compiler flag PORTABLE, so I changes it into Cmake and Docker for default value 0. In this case all of features (related to CPU) are in runtime.
So, now PORTABLE option must be:
- 0 and compiler work as best as it can
- 1 usually nothing to do; compiler default is typically the most general
- <ARCH> and we can set a CPU family for build best optimal code, e.g.: nocona core2 nehalem corei7 westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2 broadwell skylake skylake-avx512 cannonlake icelake-client rocketlake icelake-server cascadelake tigerlake cooperlake sapphirerapids alderlake bonnell atom silvermont slm goldmont goldmont-plus tremont knl knm x86-64 x86-64-v2 x86-64-v3 x86-64-v4 eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 nano-x4 k8 k8-sse3 opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx amdfam10 barcelona bdver1 bdver2 bdver3 bdver4 znver1 znver2 znver3 btver1 btver2 native
Detailed, we can see in this code diff: https://github.com/facebook/rocksdb/pull/11419/commits/5cb625142c77c84111f1f65b6f35f2318c602802
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241082974
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
If we add 0 or 1 as value (in option) cmake rewrite it into ON/OFF and only manual setup are valid
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241077014
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
I have no idea, why, but build are Ok only when we manual add -DPORTABLE=0. In all other cases:
cc1plus: error: bad value ‘OFF’ for ‘-march=’ switch
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] git-hulk commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241062554
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
Why do we need to add `PORTABLE=0` here? I saw the default value for the PORTABLE option is 0 now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] git-hulk commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241080406
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
It should be broken by this PR: https://github.com/facebook/rocksdb/pull/11419. But I think we can remove `"-DPORTABLE=0"` since the default value has been corrected to 0 in the previous line: https://github.com/apache/kvrocks/pull/1516/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR32
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241085820
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
And this way not a good point - now we can manual add cpu arch for better optimization (much better as default) and if completely remove options, there no way to do this anymore
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] git-hulk commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241085222
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
oh, I got your point. Would it be better to remove the PORTABLE option in CMakeList.txt? since the rocksdb will set the default value to 0 if the PORTABLE was not passed. cc @PragmaTwice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241085046
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
> It should be broken by this PR: [facebook/rocksdb#11419](https://github.com/facebook/rocksdb/pull/11419). But I think we can remove `"-DPORTABLE=0"` since the default value has been corrected to 0 in the previous line: https://github.com/apache/kvrocks/pull/1516/files#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR32
No, default value, if we set 0 or 1, cmake rewrite into own default ON/OFF
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] tisonkun commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241220394
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
Do we report this case to the upstream? I'm OK with this patch but we should at least report the inconvenience for possible improvement from upstream.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] tisonkun merged pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun merged PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241078651
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
As I see a cmake docs: "If no initial <value> is provided, boolean OFF is the default value", so we need a manual add 0 as value
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] tisonkun commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241301025
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
@aleksraiden Is it public and we can cross-post the link here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] tisonkun commented on pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#issuecomment-1606286120
Merging...
Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241106179
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
Yes, but we need manual setup always, because in rocksdb.cmake we use it.
So, only one right way in now - always manual set PORTABLE as a Cmake flag on build
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] aleksraiden commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "aleksraiden (via GitHub)" <gi...@apache.org>.
aleksraiden commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241227838
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
Yes, i send issue for rocksdb team
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kvrocks] git-hulk commented on a diff in pull request #1516: (deps) Bump rocksdb v8.3.2
Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1516:
URL: https://github.com/apache/kvrocks/pull/1516#discussion_r1241086940
##########
x.py:
##########
@@ -106,7 +106,7 @@ def build(dir: str, jobs: Optional[int], ghproxy: bool, ninja: bool, unittest: b
makedirs(dir, exist_ok=True)
- cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo"]
+ cmake_options = ["-DCMAKE_BUILD_TYPE=RelWithDebInfo", "-DPORTABLE=0"]
Review Comment:
What I mean is only to remove the option from the Kvrocks CMakeList.txt to prevent automatically rewriting it into ON/OFF, users still can pass the PORTABLE via the CMake arguments. Did I miss anything?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org