Adding internal config to retain non-logged stores on container start #1229
Conversation
abhishekshivanna
left a comment
abhishekshivanna
left a comment
There was a problem hiding this comment.
Thanks for these changes.
| // Internal config to clean storeDirs of a logged store on container start. This is used to benchmark bootstrap performance. | ||
| static final String CLEAN_LOGGED_STOREDIRS_ON_START = STORE_PREFIX + "%s.clean.on.container.start"; | ||
|
|
||
| // Internal config to clean storeDirs of a logged store on container start. This is used to benchmark bootstrap performance. |
There was a problem hiding this comment.
/s/logged/non-logged/
| static final String CLEAN_LOGGED_STOREDIRS_ON_START = STORE_PREFIX + "%s.clean.on.container.start"; | ||
|
|
||
| // Internal config to clean storeDirs of a logged store on container start. This is used to benchmark bootstrap performance. | ||
| static final String RETAIN_NONLOGGED_STOREDIRS_ON_START = STORE_PREFIX + "%s.retain.on.container.start"; |
There was a problem hiding this comment.
should we name this config to reflect the non-logged only case ? %s.retain.nonlogged.on.container.start
| LOG.info("Got non logged storage partition directory as " + nonLoggedStorePartitionDir.toPath().toString()); | ||
|
|
||
| if (nonLoggedStorePartitionDir.exists()) { | ||
| if (nonLoggedStorePartitionDir.exists() || !storageConfig.getRetainNonloggedStoreDirsOnStart(storeName)) { |
There was a problem hiding this comment.
update doc string to indicate that deletion is conditional.
|
@rmatharu please use PR description format. |
|
I'd prefer to not introduce force cleaning optional internal config for more combination of our stores properties (we already have one for logged store and now we have one for non-logged store). It makes it hard to follow up all the potential branching. If we still need this, can we consolidate all the flags to one configuration? Also, do we have other use cases for this outside experimental purpose? |
cameronlee314
left a comment
cameronlee314
left a comment
There was a problem hiding this comment.
Can you please add a JIRA ticket and some description about this?
What do you need this for which can't be achieved by using a logged store? It is dangerous to keep non-logged stores around, because they could have stale and incomplete data.
| LOG.info("Got non logged storage partition directory as " + nonLoggedStorePartitionDir.toPath().toString()); | ||
|
|
||
| if (nonLoggedStorePartitionDir.exists()) { | ||
| if (nonLoggedStorePartitionDir.exists() || !storageConfig.getRetainNonloggedStoreDirsOnStart(storeName)) { |
There was a problem hiding this comment.
Should this be && instead of ||?
|
Is this PR still needed? Lets close it if its not. |
6 participants
No description provided.