Skip to content

Use a separate snapshot file per lookup tier. - #5358

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

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

Conversation

@gianm

@gianm gianm commented Feb 6, 2018

Copy link
Copy Markdown
Contributor

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);

@himanshug himanshug Feb 7, 2018

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.

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
riovic918data pushed a commit to riovic918data/druid that referenced this pull request Jun 12, 2026
Prevents conflicts if two processes on the same machine use the
same lookup snapshot directory but are in different tiers.
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