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.

Sign up for GitHub

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

Jump to bottom

IGNITE-12702 Print warning when a cache value contains @AffinityKeyMapped annotation #7663

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

IGNITE-12702 Print warning when a cache value contains @AffinityKeyMapped annotation #7663

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a62c46
Merge pull request #1 from apache/master
kartiksomani Mar 29, 2020
ff382ba
Added warning logs for AffinityKeyMapped annotation on cache value
kartiksomani Apr 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 27 additions & 2 deletions .../core/src/main/java/org/apache/ignite/internal/processors/cache/IgniteCacheProxyImpl.java
View file
Open in desktop
Expand Up @@ -21,6 +21,7 @@
import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
import java.lang.reflect.Field;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
Expand All @@ -45,10 +46,12 @@
import javax.cache.processor.EntryProcessor;
import javax.cache.processor.EntryProcessorException;
import javax.cache.processor.EntryProcessorResult;

import org.apache.ignite.IgniteCache;
import org.apache.ignite.IgniteCacheRestartingException;
import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.IgniteException;
import org.apache.ignite.IgniteLogger;
import org.apache.ignite.cache.affinity.AffinityKeyMapped;
import org.apache.ignite.cache.CacheEntry;
import org.apache.ignite.cache.CacheEntryEventSerializableFilter;
import org.apache.ignite.cache.CacheEntryProcessor;
Expand Down Expand Up @@ -96,6 +99,7 @@
import org.apache.ignite.internal.util.typedef.X;
import org.apache.ignite.internal.util.typedef.internal.A;
import org.apache.ignite.internal.util.typedef.internal.CU;
import org.apache.ignite.internal.util.typedef.internal.LT;
import org.apache.ignite.internal.util.typedef.internal.S;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.lang.IgniteBiPredicate;
Expand All @@ -104,6 +108,7 @@
import org.apache.ignite.lang.IgniteProductVersion;
import org.apache.ignite.mxbean.CacheMetricsMXBean;
import org.apache.ignite.plugin.security.SecurityPermission;
import org.apache.ignite.resources.LoggerResource;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -155,6 +160,10 @@ public class IgniteCacheProxyImpl<K, V> extends AsyncSupportAdapter<IgniteCache<
*/
private final CountDownLatch initLatch = new CountDownLatch(1);

/** Injected Ignite Logger**/
@LoggerResource
private IgniteLogger log;

/**
* Empty constructor required for {@link Externalizable}.
*/
Expand All @@ -174,7 +183,6 @@ public IgniteCacheProxyImpl(
) {
this(ctx, delegate, new AtomicReference<>(null), async);
}

/**
* @param ctx Context.
* @param delegate Delegate.
Expand Down Expand Up @@ -1288,9 +1296,26 @@ public Map<K, V> getAll(Collection<? extends K> keys) {
}
}

/**
* Warns if an ineffective annotation is used
*
* @param valueCls Value class
*/
private void checkIneffectiveAnnotations(Class valueCls) {
for (Field f: valueCls.getDeclaredFields()) {
AffinityKeyMapped[] annotations = f.getDeclaredAnnotationsByType(AffinityKeyMapped.class);
if (annotations != null && annotations.length > 0) {
LT.warn(log,
"AffinityKeyMapped annotation should be used on key and not value type");
}

}
}

/** {@inheritDoc} */
@Override public void put(K key, V val) {
IgniteInternalCache<K, V> delegate = getDelegateSafe();
checkIneffectiveAnnotations(val.getClass());
Copy link
Contributor

Choose a reason for hiding this comment

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

Reflection use on each put, really? it will kill performance. Please move this check to new type registration method, I believe you will find it in BinaryMarshaller or somewhere around.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I called out the performance cost of reflection on the issue itself almost a month back. Didn't know about BinaryMarshaller, will use it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Mikhail1988 Where can I find new type registration method? I did see binary type configurator


try {
if (isAsync())
Expand Down