You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/11/03 14:25:15 UTC
[GitHub] [incubator-kvrocks] mapleFU opened a new pull request, #1069: [DNM] add batch-debugger and just testing
mapleFU opened a new pull request, #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069
Hi, here I add a tool for inspecting WriteBatch, and using it for debugging
--
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] [incubator-kvrocks] mapleFU commented on a diff in pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
mapleFU commented on code in PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#discussion_r1014152197
##########
src/storage/batch_debugger.h:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
+// This source code is licensed under both the GPLv2 (found in the
+// COPYING file in the root directory) and Apache 2.0 License
+// (found in the LICENSE.Apache file in the root directory).
+//
+// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file. See the AUTHORS file for names of contributors.
+
+#pragma once
+
+#include <rocksdb/db.h>
+#include <rocksdb/slice.h>
+
+#include <string>
+#include <vector>
+
+/// Usage:
Review Comment:
Updated 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] [incubator-kvrocks] mapleFU commented on pull request #1069: [DNM] add batch-debugger and just testing
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1302874372
@git-hulk update and making it as a tool
--
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] [incubator-kvrocks] tisonkun merged pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069
--
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] [incubator-kvrocks] caipengbo commented on a diff in pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
caipengbo commented on code in PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#discussion_r1014057108
##########
src/storage/batch_debugger.h:
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
+// This source code is licensed under both the GPLv2 (found in the
+// COPYING file in the root directory) and Apache 2.0 License
+// (found in the LICENSE.Apache file in the root directory).
+//
+// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file. See the AUTHORS file for names of contributors.
+
+#pragma once
+
+#include <rocksdb/db.h>
+#include <rocksdb/slice.h>
+
+#include <string>
+#include <vector>
+
+/// Usage:
Review Comment:
Should we write some notes to explain why the file was introduced and what the motivation was. Otherwise other developers might wonder what the file was for.
--
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] [incubator-kvrocks] mapleFU commented on pull request #1069: [DNM] add batch-debugger and just testing
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1302319004
> cool, the batch looks correct, it may be caused by the uninitialized value. Can have a look at #1068
Ok, the handler is pasted from rocksdb's `TestHandler`. It was used by rocksdb internal, so I need to copy it here. Should I just close the pull request or add `WriteBatchInspector` to our codebase?
--
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] [incubator-kvrocks] git-hulk commented on pull request #1069: [DNM] add batch-debugger and just testing
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1302325637
@mapleFU I think we can keep this PR, coz it can make the batch debug easier.
--
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] [incubator-kvrocks] tisonkun commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1304432344
Merging...
--
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] [incubator-kvrocks] mapleFU commented on pull request #1069: [DNM] add batch-debugger and just testing
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1302296044
@PragmaTwice @git-hulk
```
I20221103 15:03:19.033007 40160 replication.cc:903] LogData(3 6)Put( __namespace
7Ok/d�AA?�������, 4373)PutCF(1, __namespace
7Ok, /d�AA?��������������)
I20221103 15:03:19.033021 40160 replication.cc:903] LogData(3 6)Put( __namespace
7Ok/d�AA?�������, 4374)PutCF(1, __namespace
7Ok, /d�AA?��������������)
I20221103 15:03:19.033035 40160 replication.cc:903] LogData(3 6)Put( __namespace
7Ok/d�AA?�������, 4375)PutCF(1, __namespace
7Ok, /d�AA?��������������)
I20221103 15:03:19.033063 40160 replication.cc:903] LogData(3 6)Put( __namespace
7Ok/d�AA?�������, 4376)PutCF(1, __namespace
7Ok, /d�AA?��������������)
I20221103 15:03:19.033077 40160 replication.cc:903] LogData(3 6)Put( __namespace
7Ok/d�AA?�������, 4377)PutCF(1, __namespace
7Ok, /d�AA?��������������)
```
After inpecting hook, I think `WriteBatch::Handler` may works well, because program still coredump on `InternalKey`
--
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] [incubator-kvrocks] caipengbo commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
caipengbo commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1304365865
LGTM
--
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] [incubator-kvrocks] git-hulk commented on pull request #1069: [DNM] add batch-debugger and just testing
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1302308337
cool, the batch looks correct, it may be caused by the uninitialized value. Can have a look at #1068
--
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] [incubator-kvrocks] git-hulk commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1303580094
> @git-hulk I think it's ready to be merged now
OK, will take a look soon.
--
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] [incubator-kvrocks] tanruixiang commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1303790648
I noticed that `batch_debugger.h` was migrated from `Rocksdb`, and I don't really know if there are other specifications when there are multiple Copyrights. Do you think the current copyright notice is reasonable? @tisonkun
--
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] [incubator-kvrocks] tisonkun commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1304424709
@mapleFU it seems that we don't use this Inspector in other code?
--
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] [incubator-kvrocks] mapleFU commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1303808540
> I noticed that `batch_debugger.h` was migrated from `Rocksdb`, and I don't really know if there are other specifications when there are multiple Copyrights. Do you think the current copyright notice is reasonable?
A sample is https://github.com/facebook/rocksdb/blob/main/db/table_cache.h#L1-L8
--
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] [incubator-kvrocks] mapleFU commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1304429121
> That said, if you'd like to debug, you add the code snippet as documented?
Yes, maybe user can receive raw bytes from remote and using this to parse the input @tisonkun
--
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] [incubator-kvrocks] mapleFU commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1303312790
@git-hulk I think it's ready to be merged 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] [incubator-kvrocks] tisonkun commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1304369524
> if there are other specifications when there are multiple Copyrights
@tanruixiang in our NOTICE file there is an entry about RocksDB dependency, and RocksDB supports downstream use under the Apache License 2.0 - so basically, it won't hurt.
--
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] [incubator-kvrocks] mapleFU commented on pull request #1069: util: Add WriteBatchInspector to inspect WriteBatch
Posted by GitBox <gi...@apache.org>.
mapleFU commented on PR #1069:
URL: https://github.com/apache/incubator-kvrocks/pull/1069#issuecomment-1304420663
> But we can improve the copyright info layout and add an original link here.
@tisonkun updated, PTAL
--
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