-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-25907 Move StoreFlushContext out of HStore and make it pluggable #3298
Conversation
Change-Id: I7963a0dc65033515c1baeb4a4302f80765153635
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@@ -0,0 +1,183 @@ | |||
/* | |||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit unnecessary line
return flushContextClass.getConstructor(HStore.class, Long.class, FlushLifeCycleTracker.class) | ||
.newInstance(this, cacheFlushId, tracker); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it would be cleaner to move this into an init()
method rather than using reflection on the constructor?
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFlushContext.java
Outdated
Show resolved
Hide resolved
Change-Id: I232a805e81e4ef0a85f980e2d1503f3cf4cfe204
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked to Wellington in chat -- he said that my ask to refactor the StoreFlushContext to use Store
instead of HStore
would be a quite a bit of work.
I do think that's a good thing to do long-term, but it's not the most urgent at this moment. I'm ok to defer that for now.
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Change-Id: I492e59dd0b0f61fc4aebe9a020b4d2b679081ed5
💔 -1 overall
This message was automatically generated. |
Change-Id: I8e66c65a5d32e583038ab2e6b75803934e2c0f63
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
The previous UT failure was a flakey, have it passing locally. |
We need this to conduct other tasks related to flush without renames.