Skip to content

Use a separate snapshot file per lookup tier.#5358

Merged
jon-wei merged 1 commit intoapache:masterfrom
gianm:fix-lookups-snapshot-harder
Feb 7, 2018
Merged

Use a separate snapshot file per lookup tier.#5358
jon-wei merged 1 commit intoapache:masterfrom
gianm:fix-lookups-snapshot-harder

Conversation

@gianm
Copy link
Copy Markdown
Contributor

@gianm gianm commented Feb 6, 2018

Prevents conflicts if two processes on the same machine use the
same lookup snapshot directory but are in different tiers.

Fixes issue raised in #5344 (comment).

Prevents conflicts if two processes on the same machine use the
same lookup snapshot directory but are in different tiers.
public synchronized List<LookupBean> pullExistingSnapshot()
public synchronized List<LookupBean> pullExistingSnapshot(final String tier)
{
final File persistFile = getPersistFile(tier);
Copy link
Copy Markdown
Contributor

@himanshug himanshug Feb 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for backward compatibility...
if tier.lookupSnapshot.json is not found then we should look for lookupSnapshot.json (move that to tier.lookupSnapshot.json if exists).

let us leave some comment telling why that code exists, and, can be removed some releases after 0.12.0 .

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking backwards compatible code is not really necessary, since this feature will be able to pull lookups from the coordinator if the file is not found.

Hopefully, this is fine during a rolling upgrade, where the user is paying attention and is sure the coordinator is online and everything is healthy. Meaning snapshots aren't really needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, if coord is up then snapshot doesn't matter. so its not very critical to be backward compatible in this case.

@himanshug
Copy link
Copy Markdown
Contributor

thanks, this is great.

@jon-wei jon-wei merged commit 971d45a into apache:master Feb 7, 2018
jon-wei pushed a commit to jon-wei/druid that referenced this pull request Feb 7, 2018
Prevents conflicts if two processes on the same machine use the
same lookup snapshot directory but are in different tiers.
jon-wei pushed a commit to implydata/druid-public that referenced this pull request Feb 7, 2018
Prevents conflicts if two processes on the same machine use the
same lookup snapshot directory but are in different tiers.
leventov pushed a commit that referenced this pull request Feb 7, 2018
Prevents conflicts if two processes on the same machine use the
same lookup snapshot directory but are in different tiers.
@gianm gianm deleted the fix-lookups-snapshot-harder branch September 23, 2022 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants