Skip to content
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

MODE-1683 Simplified message consuming & receiving; updated delta reconiliation implementation and MapDB version to 0.9.8. #1032

Merged
merged 3 commits into from Jan 14, 2014

Conversation

hchiorean
Copy link
Member

This PR does not use the counter based approach from #1030. Than can be changed later on.

long lastChangeSetTime = -1;
//get all the existing records in reverse order
Records allLocalRecords = localJournal.allRecords(true);
if (allLocalRecords.size() > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does Records have an isEmpty method? Might be easier or more efficient to implement that, since we don't really care about the size here (and size could be large). Is there not a better way to find the last Record?

Copy link
Member Author

Choose a reason for hiding this comment

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

good point(s). I'll update the code.

@rhauch
Copy link
Contributor

rhauch commented Jan 14, 2014

I recently merged PR #1033 and #1034, which changed the licensing (and thus touched most of the source files) in nearly all modules except "modeshape-jcr" and those under the "sequencers" folder. Those modules have outstanding PRs, and I didn't want to create a bunch of conflicts.

There may be a conflict with deploy/jbossas/kit/jboss-eap61/org/mapdb/{0.9.7 → 0.9.8}/module.xml, but feel free to not rebase and let me clean that up.

Horia Chiorean added 2 commits January 14, 2014 09:17
…onciliation implementation and MapDB version to 0.9.8.
… Changed the signature of the methods which used long timestamps to use DateTime(s) and removed unnecessary conversions to UTC.
@hchiorean
Copy link
Member Author

Rebased on latest master and fixed conflicts. Updated the code based on the review and switched to using time-based keys.

@@ -62,6 +82,7 @@ public LocalJournal( String journalLocation,
this.asyncWritesEnabled = asyncWritesEnabled;
this.maxTimeToKeepEntriesMillis = TimeUnit.DAYS.toMillis(maxDaysToKeepEntries);
this.stopped = true;
this.searchTimeDelta = TimeUnit.SECONDS.toMillis(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this magic number coming from?

Copy link
Member Author

Choose a reason for hiding this comment

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

from the fact there will always be a slight delay locally from the point in time when a change set is created (session.save) to when it is stored in the journal (notify)

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a constant with JavaDoc that describes this?

@rhauch
Copy link
Contributor

rhauch commented Jan 14, 2014

The above comment is the only (minor) one, and it looks great.

@hchiorean
Copy link
Member Author

Amended the commit and extracted a constant.

…ed various other parts of the code based on code review.
@rhauch rhauch merged commit 280b68f into ModeShape:master Jan 14, 2014
@rhauch
Copy link
Contributor

rhauch commented Jan 14, 2014

Merged onto the 'master' branch.

@hchiorean hchiorean deleted the MODE-1683 branch January 15, 2014 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants