Repository: ignite Updated Branches: refs/heads/master 0df6eafc7 -> a58393d01
IGNITE-6701: SQL: eliminated unnecessary deserialization in GridH2Table.doUpdate. This closes #2956. Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/a58393d0 Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/a58393d0 Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/a58393d0 Branch: refs/heads/master Commit: a58393d011b2e39ead9e623e875dfd40a023ecfc Parents: 0df6eaf Author: rkondakov <[email protected]> Authored: Wed Nov 8 09:52:32 2017 +0300 Committer: devozerov <[email protected]> Committed: Wed Nov 8 09:52:32 2017 +0300 ---------------------------------------------------------------------- .../processors/query/GridQueryIndexing.java | 3 +- .../processors/query/GridQueryProcessor.java | 10 +- ...IgniteClientCacheInitializationFailTest.java | 3 +- .../query/h2/opt/GridH2SpatialIndex.java | 14 ++ .../processors/query/h2/IgniteH2Indexing.java | 10 +- .../query/h2/database/H2PkHashIndex.java | 16 ++ .../query/h2/database/H2TreeIndex.java | 23 ++- .../query/h2/opt/GridH2IndexBase.java | 15 +- .../processors/query/h2/opt/GridH2Table.java | 173 ++++++++----------- 9 files changed, 146 insertions(+), 121 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryIndexing.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryIndexing.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryIndexing.java index 7a5cbc7..4a9ee7f 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryIndexing.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryIndexing.java @@ -217,9 +217,10 @@ public interface GridQueryIndexing { * @param cctx Cache context. * @param type Type descriptor. * @param row New row. + * @param prevRow Previous row. * @throws IgniteCheckedException If failed. */ - public void store(GridCacheContext cctx, GridQueryTypeDescriptor type, CacheDataRow row) + public void store(GridCacheContext cctx, GridQueryTypeDescriptor type, CacheDataRow row, CacheDataRow prevRow) throws IgniteCheckedException; /** http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java index 8b3d832..b8c5ffa 100644 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/query/GridQueryProcessor.java @@ -1734,14 +1734,18 @@ public class GridQueryProcessor extends GridProcessorAdapter { prevRow.value(), false); - if (prevValDesc != null && prevValDesc != desc) - idx.remove(cctx, prevValDesc, prevRow); + if (prevValDesc != desc) { + if (prevValDesc != null) + idx.remove(cctx, prevValDesc, prevRow); + + prevRow = null; // Row has already been removed from another table indexes + } } if (desc == null) return; - idx.store(cctx, desc, newRow); + idx.store(cctx, desc, newRow, prevRow); } finally { busyLock.leaveBusy(); http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheInitializationFailTest.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheInitializationFailTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheInitializationFailTest.java index 9126e96..a9c8e5c 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheInitializationFailTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/IgniteClientCacheInitializationFailTest.java @@ -310,7 +310,8 @@ public class IgniteClientCacheInitializationFailTest extends GridCommonAbstractT } /** {@inheritDoc} */ - @Override public void store(GridCacheContext cctx, GridQueryTypeDescriptor type, CacheDataRow val) { + @Override public void store(GridCacheContext cctx, GridQueryTypeDescriptor type, CacheDataRow row, + CacheDataRow prevRow) { // No-op. } http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/geospatial/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2SpatialIndex.java ---------------------------------------------------------------------- diff --git a/modules/geospatial/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2SpatialIndex.java b/modules/geospatial/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2SpatialIndex.java index b6125c8..b4a8af4 100644 --- a/modules/geospatial/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2SpatialIndex.java +++ b/modules/geospatial/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2SpatialIndex.java @@ -202,6 +202,13 @@ public class GridH2SpatialIndex extends GridH2IndexBase implements SpatialIndex } } + /** {@inheritDoc} */ + @Override public boolean putx(GridH2Row row) { + GridH2Row old = put(row); + + return old != null; + } + /** * @param row Row. * @param rowId Row id. @@ -252,6 +259,13 @@ public class GridH2SpatialIndex extends GridH2IndexBase implements SpatialIndex } /** {@inheritDoc} */ + @Override public boolean removex(SearchRow row) { + GridH2Row old = remove(row); + + return old != null; + } + + /** {@inheritDoc} */ @Override public void destroy(boolean rmIndex) { Lock l = lock.writeLock(); http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java ---------------------------------------------------------------------- diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java index 4a9faa1..31902ac 100644 --- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java +++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/IgniteH2Indexing.java @@ -565,8 +565,8 @@ public class IgniteH2Indexing implements GridQueryIndexing { } /** {@inheritDoc} */ - @Override public void store(GridCacheContext cctx, GridQueryTypeDescriptor type, CacheDataRow row) - throws IgniteCheckedException { + @Override public void store(GridCacheContext cctx, GridQueryTypeDescriptor type, CacheDataRow row, + @Nullable CacheDataRow prevRow) throws IgniteCheckedException { String cacheName = cctx.name(); H2TableDescriptor tbl = tableDescriptor(schema(cacheName), cacheName, type.name()); @@ -574,7 +574,7 @@ public class IgniteH2Indexing implements GridQueryIndexing { if (tbl == null) return; // Type was rejected. - tbl.table().update(row, false); + tbl.table().update(row, prevRow); if (tbl.luceneIndex() != null) { long expireTime = row.expireTime(); @@ -603,7 +603,7 @@ public class IgniteH2Indexing implements GridQueryIndexing { if (tbl == null) return; - if (tbl.table().update(row, true)) { + if (tbl.table().remove(row)) { if (tbl.luceneIndex() != null) tbl.luceneIndex().remove(row.key()); } @@ -701,7 +701,7 @@ public class IgniteH2Indexing implements GridQueryIndexing { @Override public void apply(CacheDataRow row) throws IgniteCheckedException { GridH2Row h2Row = rowDesc.createRow(row); - h2Idx.put(h2Row); + h2Idx.putx(h2Row); } }; http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2PkHashIndex.java ---------------------------------------------------------------------- diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2PkHashIndex.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2PkHashIndex.java index be3a02e..546f5bb 100644 --- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2PkHashIndex.java +++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2PkHashIndex.java @@ -131,6 +131,14 @@ public class H2PkHashIndex extends GridH2IndexBase { } /** {@inheritDoc} */ + @Override public boolean putx(GridH2Row row) { + // Should not be called directly. Rows are inserted into underlying cache data stores. + assert false; + + throw DbException.getUnsupportedException("putx"); + } + + /** {@inheritDoc} */ @Override public GridH2Row remove(SearchRow row) { // Should not be called directly. Rows are removed from underlying cache data stores. @@ -140,6 +148,14 @@ public class H2PkHashIndex extends GridH2IndexBase { } /** {@inheritDoc} */ + @Override public boolean removex(SearchRow row) { + // Should not be called directly. Rows are removed from underlying cache data stores. + assert false; + + throw DbException.getUnsupportedException("removex"); + } + + /** {@inheritDoc} */ @Override public double getCost(Session ses, int[] masks, TableFilter[] filters, int filter, SortOrder sortOrder, HashSet<Column> allColumnsSet) { return Double.MAX_VALUE; } http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2TreeIndex.java ---------------------------------------------------------------------- diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2TreeIndex.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2TreeIndex.java index b3307d0..4ebac88 100644 --- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2TreeIndex.java +++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/database/H2TreeIndex.java @@ -205,6 +205,25 @@ public class H2TreeIndex extends GridH2IndexBase { } /** {@inheritDoc} */ + @Override public boolean putx(GridH2Row row) { + try { + InlineIndexHelper.setCurrentInlineIndexes(inlineIdxs); + + int seg = segmentForRow(row); + + H2Tree tree = treeForRead(seg); + + return tree.putx(row); + } + catch (IgniteCheckedException e) { + throw DbException.convert(e); + } + finally { + InlineIndexHelper.clearCurrentInlineIndexes(); + } + } + + /** {@inheritDoc} */ @Override public GridH2Row remove(SearchRow row) { try { InlineIndexHelper.setCurrentInlineIndexes(inlineIdxs); @@ -224,7 +243,7 @@ public class H2TreeIndex extends GridH2IndexBase { } /** {@inheritDoc} */ - @Override public void removex(SearchRow row) { + @Override public boolean removex(SearchRow row) { try { InlineIndexHelper.setCurrentInlineIndexes(inlineIdxs); @@ -232,7 +251,7 @@ public class H2TreeIndex extends GridH2IndexBase { H2Tree tree = treeForRead(seg); - tree.removex(row); + return tree.removex(row); } catch (IgniteCheckedException e) { throw DbException.convert(e); http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2IndexBase.java ---------------------------------------------------------------------- diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2IndexBase.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2IndexBase.java index 5d4a4e6..79a7e33 100644 --- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2IndexBase.java +++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2IndexBase.java @@ -199,6 +199,14 @@ public abstract class GridH2IndexBase extends BaseIndex { public abstract GridH2Row put(GridH2Row row); /** + * Puts row. + * + * @param row Row. + * @return {@code True} if existing row row has been replaced. + */ + public abstract boolean putx(GridH2Row row); + + /** * Remove row from index. * * @param row Row. @@ -207,13 +215,12 @@ public abstract class GridH2IndexBase extends BaseIndex { public abstract GridH2Row remove(SearchRow row); /** - * Remove row from index, does not return removed row. + * Removes row from index. * * @param row Row. + * @return {@code True} if row has been removed. */ - public void removex(SearchRow row) { - remove(row); - } + public abstract boolean removex(SearchRow row); /** * @param ses Session. http://git-wip-us.apache.org/repos/asf/ignite/blob/a58393d0/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java ---------------------------------------------------------------------- diff --git a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java index c299075..87e6f3d 100644 --- a/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java +++ b/modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/opt/GridH2Table.java @@ -45,7 +45,6 @@ import org.h2.index.IndexType; import org.h2.index.SpatialIndex; import org.h2.message.DbException; import org.h2.result.Row; -import org.h2.result.SearchRow; import org.h2.result.SortOrder; import org.h2.schema.SchemaObject; import org.h2.table.Column; @@ -403,37 +402,6 @@ public class GridH2Table extends TableBase { } /** - * Updates table for given key. If value is null then row with given key will be removed from table, - * otherwise value and expiration time will be updated or new row will be added. - * - * @param row Row. - * @param rmv If {@code true} then remove, else update row. - * @return {@code true} If operation succeeded. - * @throws IgniteCheckedException If failed. - */ - public boolean update(CacheDataRow row, boolean rmv) - throws IgniteCheckedException { - assert desc != null; - - GridH2Row h2Row = desc.createRow(row); - - if (rmv) - return doUpdate(h2Row, true); - else { - GridH2KeyValueRowOnheap h2Row0 = (GridH2KeyValueRowOnheap)h2Row; - - h2Row0.prepareValuesCache(); - - try { - return doUpdate(h2Row0, false); - } - finally { - h2Row0.clearValuesCache(); - } - } - } - - /** * Gets index by index. * * @param idx Index in list. @@ -453,79 +421,96 @@ public class GridH2Table extends TableBase { } /** - * For testing only. + * Updates table for given key. If value is null then row with given key will be removed from table, + * otherwise value and expiration time will be updated or new row will be added. * - * @param row Row. - * @param del If given row should be deleted from table. - * @return {@code True} if operation succeeded. + * @param row Row to be updated. + * @param prevRow Previous row. * @throws IgniteCheckedException If failed. */ - @SuppressWarnings("LockAcquiredButNotSafelyReleased") - private boolean doUpdate(final GridH2Row row, boolean del) throws IgniteCheckedException { - // Here we assume that each key can't be updated concurrently and case when different indexes - // getting updated from different threads with different rows with the same key is impossible. - lock(false); + public void update(CacheDataRow row, @Nullable CacheDataRow prevRow) + throws IgniteCheckedException { + assert desc != null; - try { - ensureNotDestroyed(); + GridH2KeyValueRowOnheap row0 = (GridH2KeyValueRowOnheap)desc.createRow(row); + GridH2KeyValueRowOnheap prevRow0 = prevRow != null ? (GridH2KeyValueRowOnheap)desc.createRow(prevRow) : null; + + row0.prepareValuesCache(); - GridH2IndexBase pk = pk(); + if (prevRow0 != null) + prevRow0.prepareValuesCache(); - if (!del) { - assert rowFactory == null || row.link() != 0 : row; + try { + lock(false); - GridH2Row old = pk.put(row); // Put to PK. + try { + ensureNotDestroyed(); - if (old == null) - size.increment(); + boolean replaced = pk().putx(row0); - int len = idxs.size(); + assert (replaced && prevRow != null) || (!replaced && prevRow == null) : "Replaced: " + replaced; - int i = pkIndexPos; + if (!replaced) + size.increment(); - // Put row if absent to all indexes sequentially. - // Start from 3 because 0 - Scan (don't need to update), 1 - PK hash (already updated), 2 - PK (already updated). - while (++i < len) { + for (int i = pkIndexPos + 1, len = idxs.size(); i < len; i++) { Index idx = idxs.get(i); if (idx instanceof GridH2IndexBase) - addToIndex((GridH2IndexBase)idx, pk, row, old, false); + addToIndex((GridH2IndexBase)idx, row0, prevRow0); } if (!tmpIdxs.isEmpty()) { for (GridH2IndexBase idx : tmpIdxs.values()) - addToIndex(idx, pk, row, old, true); + addToIndex(idx, row0, prevRow0); } } - else { - // index(1) is PK, get full row from there (search row here contains only key but no other columns). - GridH2Row old = pk.remove(row); + finally { + unlock(false); + } + } + finally { + row0.clearValuesCache(); + + if (prevRow0 != null) + prevRow0.clearValuesCache(); + } + } + + /** + * Remove row. + * + * @param row Row. + * @return {@code True} if was removed. + * @throws IgniteCheckedException If failed. + */ + public boolean remove(CacheDataRow row) throws IgniteCheckedException { + GridH2Row row0 = desc.createRow(row); + + lock(false); - if (old != null) { - // Remove row from all indexes. - // Start from 3 because 0 - Scan (don't need to update), 1 - PK hash (already updated), 2 - PK (already updated). - for (int i = pkIndexPos + 1, len = idxs.size(); i < len; i++) { - Index idx = idxs.get(i); + try { + ensureNotDestroyed(); - if (idx instanceof GridH2IndexBase) { - Row res = ((GridH2IndexBase)idx).remove(old); + boolean rmv = pk().removex(row0); - assert eq(pk, res, old) : "\n" + old + "\n" + res + "\n" + i + " -> " + index(i).getName(); - } - } + if (rmv) { + for (int i = pkIndexPos + 1, len = idxs.size(); i < len; i++) { + Index idx = idxs.get(i); - if (!tmpIdxs.isEmpty()) { - for (GridH2IndexBase idx : tmpIdxs.values()) - idx.remove(old); - } + if (idx instanceof GridH2IndexBase) + ((GridH2IndexBase)idx).removex(row0); + } - size.decrement(); + if (!tmpIdxs.isEmpty()) { + for (GridH2IndexBase idx : tmpIdxs.values()) + idx.removex(row0); } - else - return false; + + size.decrement(); } - return true; + return rmv; } finally { unlock(false); @@ -536,37 +521,15 @@ public class GridH2Table extends TableBase { * Add row to index. * * @param idx Index to add row to. - * @param pk Primary key index. * @param row Row to add to index. - * @param old Previous row state, if any. - * @param tmp {@code True} if this is proposed index which may be not consistent yet. + * @param prevRow Previous row state, if any. */ - private void addToIndex(GridH2IndexBase idx, Index pk, GridH2Row row, GridH2Row old, boolean tmp) { - assert !idx.getIndexType().isUnique() : "Unique indexes are not supported: " + idx; + private void addToIndex(GridH2IndexBase idx, GridH2Row row, GridH2Row prevRow) { + boolean replaced = idx.putx(row); - GridH2Row old2 = idx.put(row); - - if (old2 != null) { // Row was replaced in index. - if (!tmp) { - if (!eq(pk, old2, old)) - throw new IllegalStateException("Row conflict should never happen, unique indexes are " + - "not supported [idx=" + idx + ", old=" + old + ", old2=" + old2 + ']'); - } - } - else if (old != null) // Row was not replaced, need to remove manually. - idx.removex(old); - } - - /** - * Check row equality. - * - * @param pk Primary key index. - * @param r1 First row. - * @param r2 Second row. - * @return {@code true} if rows are the same. - */ - private static boolean eq(Index pk, SearchRow r1, SearchRow r2) { - return r1 == r2 || (r1 != null && r2 != null && pk.compareRows(r1, r2) == 0); + // Row was not replaced, need to remove manually. + if (!replaced && prevRow != null) + idx.removex(prevRow); } /**
