Hariharan/vbucket trait #64
Conversation
merge latest from master
|
Could you please remove the |
- flush is a variable for VBucket. - On Delete of VBucket we flush the contents in Hermes.
|
There are a lot of arbitrary white space changes. They really clutter up the PR and make it difficult to focus on the important changes. |
|
I have tried to reduce the whitespace a little. Hope that helps. |
|
Thank you. |
|
This PR triples the compilation time from ~7 seconds to ~20 seconds. Is there anything you can do to lower that? |
|
The compilation unit for Hermes is the same as you recommended. It must be due to the test case which uses catch2 framework. |
Actually, I recommended that you Catch2 compiling that slow is a deal breaker for me. Can you see if this helps? |
src/api/traits.h
Outdated
| std::string blob_name; | ||
| }; | ||
| typedef BlobInfo TraitInput; | ||
| // typedef void* TraitCallback; |
There was a problem hiding this comment.
Please remove commented out code, or add a comment explaining why it's left in.
src/api/traits.h
Outdated
|
|
||
| struct Trait; | ||
| typedef std::function<void(TraitInput &, Trait *)> TraitCallback; | ||
| // typedef void(*TraitCallback)(TraitInput &, Trait *); |
There was a problem hiding this comment.
Please remove commented out code, or add a comment explaining why it's left in.
src/api/traits.h
Outdated
| Trait(TraitID id, TraitIdArray conflict_traits, TraitType type); | ||
| }; | ||
|
|
||
| #define FILE_TRAIT 10 |
There was a problem hiding this comment.
Our convention is to prefix preprocessor symbols with HERMES_.
I have added both changes. The reason for keeping trait.cc there was because we have vBucket there as well. |
src/api/vbucket.h
Outdated
| else | ||
| std::cerr << "VBucket " << initial_name << " exists\n"; | ||
| (void)ctx; | ||
| if (IsBucketNameTooLong(name_)) { |
There was a problem hiding this comment.
Shouldn't this be IsVBucketNameTooLong?.
src/metadata_management.cc
Outdated
| VBucketID result = GetVBucketIdByName(context, rpc, name.c_str()); | ||
|
|
||
| if (result.as_int != 0) { | ||
| LOG(INFO) << "Opening Bucket '" << name << "'" << std::endl; |
There was a problem hiding this comment.
"Opening VBucket"
src/metadata_management.cc
Outdated
| LOG(INFO) << "Opening Bucket '" << name << "'" << std::endl; | ||
| IncrementRefcount(context, rpc, result); | ||
| } else { | ||
| LOG(INFO) << "Creating Bucket '" << name << "'" << std::endl; |
There was a problem hiding this comment.
"Creating VBucket"
|
Unfortunately |
|
Thanks for the changes. I'll merge this so you can make progress on the adapter, but I think we should discuss alternatives to Catch2. I'm not willing to triple our compilation times. |
Sure, we can search more if we can optimize this. Or need to switch libraries. |
This PR consists of functionality required for POSIX and STDIO adapter. Currently, It doesn't support shared traits/vBuckets as none of the metadata is stored in shared memory. I believe we will do it as we need it.