support ttl for vertex and edge #794
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #794 +/- ##
============================================
+ Coverage 69.72% 70.04% +0.31%
- Complexity 5126 5255 +129
============================================
Files 320 324 +4
Lines 24853 25433 +580
Branches 3466 3559 +93
============================================
+ Hits 17329 17814 +485
- Misses 5861 5916 +55
- Partials 1663 1703 +40
Continue to review full report at Codecov.
|
javeme
requested changes
Jan 8, 2020
| @@ -116,4 +116,8 @@ public boolean supportsNumberType() { | |||
| public boolean supportsAggregateProperty() { | |||
| return false; | |||
| } | |||
|
|
|||
| @Override public boolean supportsTtl() { | |||
| int ttl = ttl(entry); | ||
| if (ttl != 0) { | ||
| update.using(QueryBuilder.ttl(ttl)); | ||
| } |
There was a problem hiding this comment.
prefer add a static method setTtl(statment, entry)
| ImmutableMap<HugeKeys, DataType> columns = ImmutableMap.of(); | ||
| ImmutableMap<HugeKeys, DataType> columns = ImmutableMap.of( | ||
| HugeKeys.EXPIRED_TIME, DataType.decimal() | ||
| ); |
There was a problem hiding this comment.
No need to store a column of data for EXPIRED_TIME if the backend supports ttl
There was a problem hiding this comment.
we can't get ttl along with existing query statement. Using extra query degrades performance
| if (index == null) { | ||
| // Index is expired | ||
| return false; | ||
| } |
There was a problem hiding this comment.
rather determine here whether it is expired
There was a problem hiding this comment.
element_ids may have different ttl.
There was a problem hiding this comment.
prefer to change HugeIndex struct
| public static final ConfigOption<Integer> EXPIRED_DELETE_BATCH = | ||
| new ConfigOption<>( | ||
| "expired.delete_batch", | ||
| "Whether to delete expired object synchronously.", |
| if (this.deleteTemplate != null) { | ||
| protected String buildDeleteTemplate(List<HugeKeys> idNames, | ||
| Long expiredTime) { | ||
| boolean cached; |
| protected String buildDeleteTemplate(List<HugeKeys> idNames) { | ||
| if (this.deleteTemplate != null) { | ||
| protected String buildDeleteTemplate(List<HugeKeys> idNames, | ||
| Long expiredTime) { |
| } else { | ||
| cached = false; | ||
| // Set delete with ttl to avoid mistaken deletion | ||
| idNames.add(HugeKeys.EXPIRED_TIME); |
There was a problem hiding this comment.
why EXPIRED_TIME is in idNames
| @@ -118,4 +118,8 @@ public boolean supportsNumberType() { | |||
| public boolean supportsAggregateProperty() { | |||
| return false; | |||
| } | |||
|
|
|||
| @Override public boolean supportsTtl() { | |||
| @@ -45,5 +45,7 @@ palo.host=localhost | |||
|
|
|||
| snowflake.force_string=true | |||
| schema.sync_deletion=true | |||
| expired.sync_deletion=true | |||
| expired.delete_batch=1 | |||
There was a problem hiding this comment.
javeme
requested changes
Mar 7, 2020
| new ConfigOption<>( | ||
| "expired.sync_deletion", | ||
| "Whether to delete expired object synchronously.", | ||
| "sync_deletion", |
| disallowEmpty(), | ||
| false | ||
| ); | ||
|
|
||
| public static final ConfigOption<Integer> EXPIRED_DELETE_BATCH = | ||
| new ConfigOption<>( | ||
| "expired.delete_batch", | ||
| "Whether to delete expired object synchronously.", | ||
| "The batch size used to delete expired data.", |
| @@ -166,6 +171,8 @@ public void init(CassandraSessionPool.Session session) { | |||
| .put(HugeKeys.ENABLE_LABEL_INDEX, DataType.cboolean()) | |||
| .put(HugeKeys.USER_DATA, TYPE_UD) | |||
| .put(HugeKeys.STATUS, DataType.tinyint()) | |||
| .put(HugeKeys.TTL, DataType.cint()) | |||
| @@ -294,7 +301,8 @@ public void init(CassandraSessionPool.Session session) { | |||
| HugeKeys.OTHER_VERTEX, TYPE_ID | |||
| ); | |||
| ImmutableMap<HugeKeys, DataType> columns = ImmutableMap.of( | |||
| HugeKeys.PROPERTIES, DataType.map(TYPE_PK, TYPE_PROP) | |||
| HugeKeys.PROPERTIES, DataType.map(TYPE_PK, TYPE_PROP), | |||
| HugeKeys.EXPIRED_TIME, DataType.decimal() | |||
There was a problem hiding this comment.
define TYPE_EXPIRED_TIME as long
| ImmutableMap<HugeKeys, DataType> columns = ImmutableMap.of(); | ||
| ImmutableMap<HugeKeys, DataType> columns = ImmutableMap.of( | ||
| HugeKeys.EXPIRED_TIME, DataType.decimal() | ||
| ); |
| if (!graph.graphTransaction().store().features().supportsTtl() && | ||
| !query.showExpired() && | ||
| index.expiredTime() != 0L && index.expiredTime() < now) { | ||
| GraphTransaction.asyncDeleteExpiredObject(graph, index); |
| @@ -437,5 +437,9 @@ public boolean supportsNumberType() { | |||
| public boolean supportsAggregateProperty() { | |||
| return false; | |||
| } | |||
|
|
|||
| @Override public boolean supportsTtl() { | |||
There was a problem hiding this comment.
wrap line, check all "@OverRide public"
| @@ -189,6 +193,10 @@ protected void updateIndex(Id ilId, HugeElement element, boolean removed) { | |||
| // Not build index for record with nullable field except unique index | |||
| List<Object> propValues = allPropValues.subList(0, firstNullField); | |||
|
|
|||
| // Expired time | |||
| long expiredTime = element.type().isEdge() ? | |||
There was a problem hiding this comment.
maybe it's better to keep compatible with Vertex
| if (index == null) { | ||
| // Index is expired | ||
| return false; | ||
| } |
There was a problem hiding this comment.
prefer to change HugeIndex struct
| protected String buildDeleteTemplate(List<HugeKeys> idNames, | ||
| Long expiredTime) { | ||
| boolean updateCache; | ||
| if (expiredTime == null || expiredTime == 0L) { |
There was a problem hiding this comment.
have to use expiredTime == null?
zhoney
force-pushed
the
ttl
branch
2 times, most recently
from
481d343
to
be2c284
Compare
javeme
requested changes
Mar 17, 2020
|
|
||
| public Set<IdWithExpiredTime> expiredElementIds() { | ||
| long now = DateUtil.now().getTime(); | ||
| Set<IdWithExpiredTime> expired = new LinkedHashSet<>(); |
There was a problem hiding this comment.
InsertionOrderUtil.newSet(this.elementIds.size())
| public Set<Id> elementIds() { | ||
| return Collections.unmodifiableSet(this.elementIds); | ||
| Set<Id> ids = new LinkedHashSet<>(this.elementIds.size()); |
There was a problem hiding this comment.
InsertionOrderUtil.newSet(this.elementIds.size())
I will add method InsertionOrderUtil.newSet(int size) to common
| @@ -187,7 +188,7 @@ private void unlistenChanges() { | |||
|
|
|||
| return results; | |||
| } | |||
|
|
|||
| @SuppressWarnings("unchecked") | |||
| if (survivedEdges.size() < edges.size()) { | ||
| edges = survivedEdges; | ||
| } | ||
| if (edges.size() <= MAX_CACHE_EDGES_PER_QUERY) { |
There was a problem hiding this comment.
don't update cache if unneeded
| @@ -569,6 +585,20 @@ private IdHolder doIndexQueryBatch(IndexLabel indexLabel, | |||
| }); | |||
| } | |||
|
|
|||
| private void removeExpiredIndexIfNeeded(HugeIndex index, | |||
| boolean showExpired) { | |||
| if (graph().graphTransaction().store().features().supportsTtl() || | |||
| @@ -169,17 +184,34 @@ protected void parseRowColumns(Result row, BackendEntry entry, | |||
| public static class IndexTable extends HbaseTable { | |||
|
|
|||
| private static final int INDEX_DELETE_BATCH = 1000; | |||
| protected HugeType type; | |||
| session.put(this.table(), CF, col.name, | ||
| BinarySerializer.EMPTY_BYTES, col.value); | ||
| BytesBuffer buffer = BytesBuffer.wrap(col.name); | ||
| buffer.readIndexId(this.type()); |
There was a problem hiding this comment.
add a field ttl to BackendColumn
| protected String buildDeleteTemplate(List<HugeKeys> idNames, | ||
| Long expiredTime) { | ||
| boolean updateCache; | ||
| if (expiredTime == null || expiredTime == 0L) { |
| } else { | ||
| cached = false; | ||
| // Set delete with ttl to avoid mistaken deletion | ||
| idNames.add(HugeKeys.EXPIRED_TIME); |
| List<HugeKeys> idNames = this.idColumnName(); | ||
| String template = this.buildDeleteTemplate(idNames); | ||
|
|
||
| List<HugeKeys> idNames = new ArrayList<>(this.idColumnName()); |
javeme
requested changes
Apr 20, 2020
| @@ -101,7 +101,7 @@ | |||
| * [0.50] Issue-746: Support userdata for index label | |||
| * [0.51] Issue-929: Support 5 TP RESTful API | |||
| * [0.52] Issue-781: Support range query for rest api like P.gt(18) | |||
| * [0.53] Issue-295: Support ttl for edge label | |||
| * [0.53] Issue-295: Support ttl | |||
There was a problem hiding this comment.
Support ttl for edge and vertex
| @Override | ||
| public void eliminate(CassandraSessionPool.Session session, | ||
| CassandraBackendEntry.Row entry) { | ||
| Update append = this.buildEliminate(entry); |
There was a problem hiding this comment.
typo: append
and seems no need to set ttl for eliminate
| Using usingTtl = QueryBuilder.ttl(ttl); | ||
| long ttl = entry.ttl(); | ||
| if (ttl != 0L) { | ||
| int calcTtl = (int) Math.ceil((double) ttl / 1000); |
There was a problem hiding this comment.
convert (double) ttl may loss accuracy, prefer ttl / 1000d
| @@ -655,6 +660,8 @@ public static void shutdown(long timeout) { | |||
| private final AtomicInteger refs; | |||
| // Flag opened of each thread | |||
| private final ThreadLocal<Boolean> opened; | |||
| // Last time check flag opened of each thread | |||
| private final ThreadLocal<Long> lastCheckOpenedTime; | |||
| @@ -707,6 +716,7 @@ public void rollback() { | |||
|
|
|||
| @Override | |||
| public boolean isOpen() { | |||
| this.lastCheckOpenedTime.set(DateUtil.now().getTime()); | |||
| private Set<Id> elementIds; | ||
| private long expiredTime; | ||
| private Set<IdWithExpiredTime> elementIds; | ||
| private HugeGraph graph; |
There was a problem hiding this comment.
set final and move to the first line
| @Override | ||
| public void insert(Session session, BackendEntry entry) { | ||
| long ttl = entry.ttl(); | ||
| if (ttl == 0) { |
| long expiredTime = BytesBuffer.wrap(col.value).readVLong(); | ||
| if (expiredTime == 0) { | ||
| long ttl = entry.ttl(); | ||
| if (ttl == 0) { |
| long expiredTime = buffer.readVLong(); | ||
| if (expiredTime == 0) { | ||
| long ttl = entry.ttl(); | ||
| if (ttl == 0) { |
| if (aggregate == null && !hasOrder) { | ||
| select.append(this.orderByKeys()); | ||
| } | ||
| if (!query.nolimit() || query.offset() > 0) { |
javeme
requested changes
Apr 22, 2020
| IdQuery newQuery = new IdQuery(HugeType.VERTEX, query); | ||
| List<HugeVertex> vertices = new ArrayList<>(); | ||
| for (Id vertexId : query.ids()) { | ||
| HugeVertex vertex = (HugeVertex) this.verticesCache.get(vertexId); | ||
| if (vertex == null) { | ||
| if (vertex == null || vertex.expired()) { |
| @@ -716,7 +712,9 @@ public void rollback() { | |||
|
|
|||
| @Override | |||
| public boolean isOpen() { | |||
| this.lastCheckOpenedTime.set(DateUtil.now().getTime()); | |||
| if (this.transactions.get() != null) { | |||
| this.transactions.get().openTime(DateUtil.now().getTime()); | |||
zhoney
force-pushed
the
ttl
branch
3 times, most recently
from
c47e73c
to
df01dc7
Compare
implemented: #295 Change-Id: I16a762c5aa850f14cbd554d29fb8592db87e7619
Change-Id: I8a2a7869136908eb86f9330cbf807eb24a157717
Change-Id: I29f4cfa2fca4b52e0c6a20edaa0885b51e1d2c57
Change-Id: I08e85edbe872480d6bf37019caa832c30d0d8735
Change-Id: I076ba069123ae7e9530d5a5e2f3c6054c781b7a4
Change-Id: If953f7321a424e62c73fea90969588cd8ecb8db6
Change-Id: I2b83062d46906165e9ad8dee385d39fd1c7b71fb
Change-Id: Ie8be022cd19caf53aa59ab65c12266e109d911dc
Change-Id: Ieaff6f73dac21b73696e529f0ba65163239de7b6
Change-Id: Ib64d259c6f4eb67b370b4261a93c40f77fe4a6b1
Change-Id: I886df8c99e80a9514bb6076deac5dca5da177569
Change-Id: Ia00f4811b27048629497d476a25dfc7c5c60470b
Change-Id: Ib3afe8ca6f76d761ba8f0c7140af31ce6c1cbb6c
Change-Id: Iba66cde9ae8152e47830ec6ec9034816a3cc3f7d
Change-Id: I363947c8031ae150a6e769255bc54e0d0db586cb
Change-Id: Ibab2e6034a45cbf808a5a6718c1e42704844cb74
Change-Id: I1b1832e2a55b4587e3a1521d0409765cf849f9f8
Change-Id: Id0efc2bac16fea21f41b9e348ed9d6684ee8034f
javeme
approved these changes
May 13, 2020
imbajin
reviewed
May 14, 2020
Comment on lines
+558
to
+579
| <<<<<<< HEAD | ||
| ======= | ||
| * Add a row record to a table with ttl for index | ||
| */ | ||
| public void put(String table, byte[] family, byte[] rowkey, | ||
| byte[] qualifier, byte[] value, long ttl) { | ||
| Put put = new Put(rowkey); | ||
| put.addColumn(family, qualifier, value); | ||
| put.setTTL(ttl); | ||
| this.batch(table, put); | ||
| } | ||
|
|
||
| /** | ||
| * Delete a record by rowkey and qualifier from a table | ||
| */ | ||
| public void remove(String table, byte[] family, | ||
| byte[] rowkey, byte[] qualifier) { | ||
| this.remove(table, family, rowkey, qualifier, false); | ||
| } | ||
|
|
||
| /** | ||
| >>>>>>> support edgeLabel ttl |
There was a problem hiding this comment.
Seems there is a pair of neglected conflicting code block marks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
implemented: #295
Change-Id: I16a762c5aa850f14cbd554d29fb8592db87e7619