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

Adding simple cache wrapper around index sets list in MongoIndexSetRegistry. #3440

Merged
merged 4 commits into from Jan 30, 2017

Conversation

Projects
None yet
2 participants
@dennisoelkers
Member

dennisoelkers commented Jan 27, 2017

Description

Motivation and Context

Before this change, every flush of the batched ES output results in MongoDB hits twice, because of status checks for all writable targets of all index sets.

This change adds a simple single-element cache in the MongoIndexSetRegistry class, which keeps

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

dennisoelkers added some commits Jan 27, 2017

@dennisoelkers dennisoelkers added the bug label Jan 27, 2017

@dennisoelkers dennisoelkers added this to the 2.2.0 milestone Jan 27, 2017

@joschi joschi self-assigned this Jan 30, 2017

public class MongoIndexSetRegistry implements IndexSetRegistry {
private final IndexSetService indexSetService;
private final MongoIndexSet.Factory mongoIndexSetFactory;
static class IndexSetsCache {
private final IndexSetService indexSetService;
private AtomicReference<Supplier<List<IndexSetConfig>>> indexSetConfigs;

This comment has been minimized.

@joschi

joschi Jan 30, 2017

Contributor

Why do we need a Supplier<T> here? Wouldn't an atomic reference to any List<IndexSetConfig> (e. g. ImmutableList<IndexSetConfig> be sufficient since we only operate (replace or return) the complete list instead of individual items?

This comment has been minimized.

@dennisoelkers

dennisoelkers Jan 30, 2017

Member

Using the supplier guarantees it is refreshed lazily, to prevent every node hammering the collection after a change event.

This comment has been minimized.

@joschi

joschi Jan 30, 2017

Contributor

Good point. 👍

}
List<IndexSetConfig> get() {
return indexSetConfigs.get().get();

This comment has been minimized.

@joschi

joschi Jan 30, 2017

Contributor

Maybe returning a copy of the list would be useful here.

This comment has been minimized.

@dennisoelkers

dennisoelkers added some commits Jan 30, 2017

@joschi

joschi approved these changes Jan 30, 2017

@joschi joschi merged commit 55dab59 into master Jan 30, 2017

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details

@joschi joschi deleted the issue-3433 branch Jan 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment