6.2 upgrade checklist #37
Comments
|
I marked the optimization to skip over sstables that are fully covered by range tombstones as dropped due to time constraints. I wasn't able to come up with a clean implementation of |
Ack. Ok. |
|
I was able to compile on Mac without feeaa41. Not sure what to do with it - maybe try dropping it and see if anyone else's build breaks? It is hard to submit upstream right now since I can't say what it fixes. |
|
Were you able to link the Cockroach binary without that patch, or just
build RocksDB? If I recall correctly things don’t go wrong until you try to
link Cockroach.
…On Thu, Jun 27, 2019 at 2:24 PM Andrew Kryczka ***@***.***> wrote:
I was able to compile on Mac without feeaa41
<feeaa41>.
Not sure what to do with it - maybe try dropping it and see if anyone
else's build breaks? It is hard to submit upstream right now since I can't
say what it fixes.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#37?email_source=notifications&email_token=AAGXSICI3L4SXOOSTLTIRVLP4UAVVA5CNFSM4H3WYHH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYX66QQ#issuecomment-506457922>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGXSIAOPHK73DJRKG5UCY3P4UAVVANCNFSM4H3WYHHQ>
.
|
|
Thanks Nikhil :). It did appear to successfully link the cockroach binary. I could have made a mistake or could have been using different dependency versions (I was running on qemu on Linux as I don't have a real Mac). Let me try soliciting someone who actually uses Mac to help test. |
|
Ah, gotcha. So if, using the current master, I run But perhaps that's been fixed somehow in RocksDB 6.2! |
|
I see. Thanks for the build output. The candidate 6.2 release branch is here if you want to try: https://github.com/cockroachdb/rocksdb/tree/crl-release-6.2.1 |
|
I did try! But it looks like some changes are needed in lib roach, too? Is
there a corresponding patch for cockroachdb/cockroach that you've got handy?
…On Thu, Jun 27, 2019 at 2:41 PM Andrew Kryczka ***@***.***> wrote:
I see. Thanks for the build output. The candidate 6.2 release branch is
here if you want to try:
https://github.com/cockroachdb/rocksdb/tree/crl-release-6.2.1
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#37?email_source=notifications&email_token=AAGXSIGF27QCXYGTOEY72ZDP4UCWNA5CNFSM4H3WYHH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYYAPPY#issuecomment-506464191>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAGXSICKOHL62JHTZEYGUXTP4UCWNANCNFSM4H3WYHHQ>
.
|
|
Oh really? Do you mind sharing the compiler error? I am building cockroach master branch against that rocksdb 6.2 branch and it seems to work. |
|
Never mind. TIL |
D'oh. Perhaps the rule for deleting So your patch fixes the libroach compilation failure—thanks! I tested building the latest master with RocksDB pointed at crl-release-6.2.1 and got a jemalloc related error: That error is indeed resolved by adding feeaa41 to the RocksDB submodule, albeit a bit modified for some upstream changes: diff --git a/db/malloc_stats.cc b/db/malloc_stats.cc
index bcee5c3fb..1dfe0d55b 100644
--- a/db/malloc_stats.cc
+++ b/db/malloc_stats.cc
@@ -20,10 +20,6 @@ namespace rocksdb {
#ifdef ROCKSDB_JEMALLOC
-#ifdef JEMALLOC_NO_RENAME
-#define malloc_stats_print je_malloc_stats_print
-#endif
-
typedef struct {
char* cur;
char* end;
diff --git a/port/jemalloc_helper.h b/port/jemalloc_helper.h
index 0c216face..e5472ff70 100644
--- a/port/jemalloc_helper.h
+++ b/port/jemalloc_helper.h
@@ -9,6 +9,7 @@
#ifdef __FreeBSD__
#include <malloc_np.h>
#else
+#define JEMALLOC_MANGLE
#include <jemalloc/jemalloc.h>
#endif
The rationale, if you're interested in upstreaming this patch, is that the current RocksDB code is broken when linking against a jemalloc that's been configured with a non-empty Anyway, it seems like it's worth working with jemallocs that have been built with |
|
Understood, thanks a lot for your help! Will submit upstream. |
|
Heh, thanks for doing all the heavy lifting! |
Release branch: https://github.com/cockroachdb/rocksdb/tree/crl-release-6.2.1 Checklist: cockroachdb/rocksdb#37 Fixes cockroachdb#37602 Release note: None
38576: c-deps: upgrade rocksdb to 6.2.1 r=ajkr a=ajkr Release branch: https://github.com/cockroachdb/rocksdb/tree/crl-release-6.2.1 See checklist: cockroachdb/rocksdb#37 Fixes #37602 Release note: None Co-authored-by: Andrew Kryczka <andrew.kryczka2@gmail.com>
Release branch: https://github.com/cockroachdb/rocksdb/tree/crl-release-6.2.1 Checklist: cockroachdb/rocksdb#37 Fixes cockroachdb#37602 Release note: None
38781: c-deps: upgrade rocksdb to 6.2.1 r=ajkr a=ajkr This is a resubmission of c505411 which got accidentally reverted in f8aaa90. Release branch: https://github.com/cockroachdb/rocksdb/tree/crl-release-6.2.1 Checklist: cockroachdb/rocksdb#37 Fixes #37602 Release note: None Co-authored-by: Andrew Kryczka <andrew.kryczka2@gmail.com>
checklist generation command:
branch: https://github.com/cockroachdb/rocksdb/tree/crl-release-6.2.1
checklist:
--with-jemalloc-prefixfacebook/rocksdb#5521)fallocateeven if hole-punching unsupported Original upstream commit 186b3af is presenttotal_order_seekfor scans Original upstream commit 5a5c049 is presentSyncClosedLogsfor multiple CFs (OnlySyncClosedLogsfor multiple CFs facebook/rocksdb#4460) Original upstream commit a478682 is presentThe text was updated successfully, but these errors were encountered: