Skip to content

Conversation

@ahgittin
Copy link
Contributor

the persists and deletions run simultaneously; now, don't write things which are deleted.
also suppress the immediate delta write that was previously being done;
this may impact tests if there relying on an immediate checkpoint, but that's a bad test,
for in normal operation checkpointing should be started when the mgmt node starts and there should be nothing to persist.
if we support turning on persistence later that should trigger a full write not a delta write.

…tem not to be deleted

the persists and deletions run simultaneously; now, don't write things which are deleted.
also suppress the immediate delta write that was previously being done;
this may impact tests if there relying on an immediate checkpoint, but that's a bad test,
for in normal operation checkpointing should be started when the mgmt node starts and there should be nothing to persist.
if we support turning on persistence later that should trigger a full write not a delta write.
@ahgittin
Copy link
Contributor Author

failure unrelated, infra problem https://issues.apache.org/jira/browse/INFRA-11316

Stopwatch stopwatch = Stopwatch.createStarted();
List<ListenableFuture<?>> futures = Lists.newArrayList();

Set<String> deletedIds = MutableSet.of();
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised we need this. I can see that if one uses ImmediateDeltaChangeListener then it would happen - but that class is deprecated and not used (we should delete it).

If using PeriodicDeltaChangeListener.persistNowInternal() (which I thought all code paths would do), then it has already gone through this to remove the objects whose ids were removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like persistNowInternal doesn't actually do that, that's why we need it

@ahgittin
Copy link
Contributor Author

ahgittin commented Mar 4, 2016

the problem is real so i'm going to merge but will review more after

@asfgit asfgit merged commit 7b0ee55 into apache:master Mar 4, 2016
asfgit pushed a commit that referenced this pull request Mar 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants