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

Use an explicit per-View modification counter [ECR-2804] #658

Merged
merged 11 commits into from
Jan 18, 2019
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
### Removed
- `com.exonum.binding.common.proofs.map.MapEntry` — moved to package
`com.exonum.binding.common.collect`.
- `ViewModificationCounter` replaced with per-`View` modification counters to simplify
their relationship and testing. (#658)

### Fixed
- A bug in the cryptocurrency demo frontend that sometimes resulted in rejected transactions and/or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,17 @@

import static com.google.common.base.Preconditions.checkNotNull;

import com.exonum.binding.proxy.CleanAction;
import com.exonum.binding.proxy.Cleaner;
import com.exonum.binding.proxy.NativeHandle;
import com.exonum.binding.proxy.ProxyDestructor;

/**
* A fork is a database view, allowing both read and write operations.
*
* <p>A fork allows to perform a transaction: a number of independent writes to a database,
* which then may be <em>atomically</em> applied to the database state.
* <p>A fork allows to perform
* a {@linkplain com.exonum.binding.transaction.Transaction transaction}: a number of independent
* writes to the database, which then may be <em>atomically</em> applied (i.e. committed)
* to the database and change the database state.
*/
public final class Fork extends View {

Expand Down Expand Up @@ -60,15 +61,7 @@ public static Fork newInstance(long nativeHandle, boolean owningHandle, Cleaner
}
});

// Create the fork
Fork f = new Fork(h, cleaner);

// Add the action that unregisters the fork separately so that it is always invoked.
cleaner.add(CleanAction.from(() -> ViewModificationCounter.getInstance().remove(f),
"Fork in modification counter")
);

return f;
return new Fork(h, cleaner);
}

/**
Expand All @@ -77,6 +70,6 @@ public static Fork newInstance(long nativeHandle, boolean owningHandle, Cleaner
* @param nativeHandle a handle of the native Fork object
*/
private Fork(NativeHandle nativeHandle, Cleaner cleaner) {
super(nativeHandle, cleaner);
super(nativeHandle, cleaner, new IncrementalModificationCounter());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright 2019 The Exonum Team
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.exonum.binding.storage.database;

enum ImmutableModificationCounter implements ModificationCounter {

INSTANCE;

private static final int INITIAL_VALUE = 0;

@Override
public boolean isModifiedSince(int lastValue) {
return false;
}

@Override
public int getCurrentValue() {
return INITIAL_VALUE;
}

@Override
public void notifyModified() {
throw new IllegalStateException("Immutable counter cannot be modified");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright 2019 The Exonum Team
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.exonum.binding.storage.database;

final class IncrementalModificationCounter implements ModificationCounter {

private int counter = 0;

@Override
public boolean isModifiedSince(int lastValue) {
return counter != lastValue;
}

@Override
public int getCurrentValue() {
return counter;
}

@Override
public void notifyModified() {
counter++;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright 2019 The Exonum Team
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.exonum.binding.storage.database;

/**
* A counter of modification events of some objects (e.g., a collection, or a database view).
* It is updated each time the object notifies of an event. The clients that need
* to detect modifications must save the current value of the counter, and check if it has changed
* to determine if the corresponding source object is modified.
*
* <p>Implementations must reliably detect up to 4 billion modifications (2^32-1).
*
* <p>Implementations are not required to be thread-safe.
*/
public interface ModificationCounter {

/**
* Returns true if the counter was modified since the given value (if {@link #notifyModified()}
* has been invoked); false — otherwise.
*
* @param lastValue the last value of the counter
*/
boolean isModifiedSince(int lastValue);

/**
* Returns the current value of the counter. No assumptions must be made on how it changes
* when a notification is received.
*/
int getCurrentValue();

/**
* Notifies this counter that the source object is modified, updating its current value.
*
* @throws IllegalStateException if this counter corresponds to a read-only (immutable) object,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if a counter shall concern itself with immutable things (on top of that, the AbstractIndexProxy does this check itself)

Copy link
Contributor

@MakarovS MakarovS Jan 16, 2019

Choose a reason for hiding this comment

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

I think it's better to be safe and throw an exception in case of an immutable counter (as it is now)

* i.e., must reject any modification events
*/
void notifyModified();
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,6 @@ public static Snapshot newInstance(long nativeHandle, boolean owningHandle, Clea
}

private Snapshot(NativeHandle nativeHandle, Cleaner cleaner) {
super(nativeHandle, cleaner);
super(nativeHandle, cleaner, ImmutableModificationCounter.INSTANCE);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,28 @@
* <li>A fork, which is a <em>read-write</em> view.</li>
* </ul>
*
* <p>As in some cases the clients need to detect any changes made to a database, a view also
* holds a modification counter, which any clients changing the database state must notify.
*
* @see Snapshot
* @see Fork
*/
public abstract class View extends AbstractNativeProxy {

private final Cleaner cleaner;
private final ModificationCounter modCounter;

/**
* Create a new view proxy.
*
* @param nativeHandle a native handle: an implementation-specific reference to a native object
* @param cleaner a cleaner of resources
* @param modCounter a modification counter
*/
View(NativeHandle nativeHandle, Cleaner cleaner) {
View(NativeHandle nativeHandle, Cleaner cleaner, ModificationCounter modCounter) {
super(nativeHandle);
this.cleaner = cleaner;
this.modCounter = modCounter;
}

/**
Expand All @@ -62,4 +68,12 @@ public long getViewNativeHandle() {
public Cleaner getCleaner() {
return cleaner;
}

/**
* Returns a modification counter, corresponding to this view. The clients, trying to modify
* a collection using this view, must notify the counter first.
*/
public ModificationCounter getModificationCounter() {
return modCounter;
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
import com.exonum.binding.proxy.AbstractNativeProxy;
import com.exonum.binding.proxy.NativeHandle;
import com.exonum.binding.storage.database.Fork;
import com.exonum.binding.storage.database.ModificationCounter;
import com.exonum.binding.storage.database.View;
import com.exonum.binding.storage.database.ViewModificationCounter;

/**
* An abstract super class for proxies of all indices.
Expand All @@ -38,7 +38,7 @@ abstract class AbstractIndexProxy extends AbstractNativeProxy implements Storage
/**
* Needed to detect modifications of this index during iteration over this (or other) indices.
*/
final ViewModificationCounter modCounter;
final ModificationCounter modCounter;

private final String name;

Expand All @@ -56,7 +56,7 @@ abstract class AbstractIndexProxy extends AbstractNativeProxy implements Storage
super(nativeHandle);
this.name = checkIndexName(name);
this.dbView = checkNotNull(view);
this.modCounter = ViewModificationCounter.getInstance();
this.modCounter = view.getModificationCounter();
}

/** Returns the name of this index. */
Expand All @@ -71,21 +71,20 @@ public final String getName() {
* @throws UnsupportedOperationException if the database view is read-only
*/
void notifyModified() {
modCounter.notifyModified(castViewToFork());
checkCanModify();
modCounter.notifyModified();
}

/**
* Checks that a database view is an instance of {@link Fork} — a modifiable database view.
*
* @return a modifiable view: a Fork.
* @throws UnsupportedOperationException if view is read-only or null.
*/
private Fork castViewToFork() {
private void checkCanModify() {
if (!(dbView instanceof Fork)) {
throw new UnsupportedOperationException("Cannot modify the view: " + dbView
+ "\nUse a Fork to modify any collection.");
}
return (Fork) dbView;
}

@Override
Expand Down
Loading