Skip to content

Geowave 1027 - Accumulo Bulk Delete#1077

Merged
rfecher merged 10 commits intomasterfrom
geowave-1027-kam
Apr 27, 2017
Merged

Geowave 1027 - Accumulo Bulk Delete#1077
rfecher merged 10 commits intomasterfrom
geowave-1027-kam

Conversation

@blastarr
Copy link
Copy Markdown
Contributor

No description provided.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 20, 2017

Coverage Status

Coverage increased (+0.2%) to 52.952% when pulling 42e06a1 on geowave-1027-kam into 1df498f on master.

2 similar comments
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 52.952% when pulling 42e06a1 on geowave-1027-kam into 1df498f on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 52.952% when pulling 42e06a1 on geowave-1027-kam into 1df498f on master.


private final AccumuloSplitsProvider splitsProvider = new AccumuloSplitsProvider();

private final HashMap<ByteArrayId, Integer> dupCountMap = new HashMap<>();
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.

these maps cannot be used per datastore - delete() should be able to be called concurrently with the same instance of a datastore (no shared state).

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.

OK. Can we discuss?


protected boolean useWholeRowIterator() {
return (visibilityCounts == null) || visibilityCounts.isAnyEntryDifferingFieldVisiblity();
return false;// (visibilityCounts == null) ||
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.

what's going on here?

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.

Not my edit.

Comment thread deploy/pom.xml Outdated
<artifactId>geowave-datastore-hbase</artifactId>
<version>${project.version}</version>
</dependency>
<!--
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.

probably cleaner to remove commented out block here

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.

OK.

@@ -0,0 +1,92 @@
package mil.nga.giat.geowave.cli.debug;
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.

is this duplicative of vector CQLDelete? Is there a reason to have this separately?

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.

No. This can go away now that 'vector cqldelete' is working


public static Range byteArrayRangeToAccumuloRange(
final ByteArrayRange byteArrayRange ) {
if (byteArrayRange.isSingleValue()) {
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.

out of curiousity, did this relate to a bug encountered? ie. how did you come across this change?

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.

Not my edit.

final DistributableQuery query = TestUtils.resourceToQuery(savedFilterResource);
CloseableIterator<?> actualResults;

actualResults = geowaveStore.query(
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.

to test dupe bookkeeping, use a null query to count all the results as well as this to count how many should be deleted...after delete query all the results again and make sure it equals initialAllResults - expectedFeaturesDeleted

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.

OK

@blastarr
Copy link
Copy Markdown
Contributor Author

@rfecher - Hold off on the PR - i made some improvements to the ITs, and i'll need to squash again after these last three commits I think

@blastarr
Copy link
Copy Markdown
Contributor Author

@rfecher - I'm done w/ my edits, but we still have those two that weren't mine. Need to decide why those are there and what (if anything) we should do about them.

@blastarr
Copy link
Copy Markdown
Contributor Author

@rfecher - Also, do we want to squash ALL the 1027 commits once the edits are complete?

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 21, 2017

Coverage Status

Coverage increased (+0.2%) to 52.976% when pulling be76829 on geowave-1027-kam into 1df498f on master.

5 similar comments
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 52.976% when pulling be76829 on geowave-1027-kam into 1df498f on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 52.976% when pulling be76829 on geowave-1027-kam into 1df498f on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 52.976% when pulling be76829 on geowave-1027-kam into 1df498f on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 52.976% when pulling be76829 on geowave-1027-kam into 1df498f on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.2%) to 52.976% when pulling be76829 on geowave-1027-kam into 1df498f on master.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 21, 2017

Coverage Status

Coverage increased (+0.2%) to 52.986% when pulling be76829 on geowave-1027-kam into 1df498f on master.

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 21, 2017

Coverage Status

Coverage increased (+0.2%) to 52.979% when pulling 1db253f on geowave-1027-kam into 1df498f on master.

@rfecher rfecher merged commit 546d221 into master Apr 27, 2017
@rfecher rfecher deleted the geowave-1027-kam branch April 27, 2017 17:08
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