-
Notifications
You must be signed in to change notification settings - Fork 526
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
HDDS-12582. TypedTable support using different codec #8073
Conversation
@sumitagrawl , if there is a more efficient codec, we should replace the existing codec with it instead adding this custom codec feature. |
Efficiency here is based on usecase, Suppose for iteration, if Value is not required to be deserialized, can have codec of not doing anything. But default always deserialize. Further, can have different target of deserialization, like,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitagrawl , thanks for working on this! This approach "different tables could use a different codec" is better than "custom ValueCodec". Please update also the title.
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/RDBStore.java
Show resolved
Hide resolved
@@ -87,32 +88,43 @@ public TypedTable(RDBTable rawTable, | |||
* @param keyType The key type. | |||
* @param valueType The value type. | |||
* @param cacheType How to cache the entries? | |||
* @param threadNamePrefix | |||
* @param threadNamePrefix the thread name prefix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change threadNamePrefix
; see HDDS-12590.
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/cache/TableNoCache.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/cache/TableNoCache.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/cache/TableNoCache.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/cache/TableNoCache.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java
Show resolved
Hide resolved
@sumitagrawl , #8076 is merged now. Please resolve the conflicts. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitagrawl , thanks a lot for the update! Please see the comments inlined.
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/cache/TableNoCache.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/cache/TableNoCache.java
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/db/TypedTable.java
Show resolved
Hide resolved
1c46f23
to
0328bf1
Compare
@szetszwo to resolve conflict, reapply the changes is done on master with threadNamePrefix; see HDDS-12590 as merged |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 the change looks good.
Just a very minor inlined. I am fine if we keep the code as-is.
@@ -34,6 +34,7 @@ | |||
@Private | |||
@Evolving | |||
public interface TableCache<KEY, VALUE> { | |||
CacheResult<?> MAY_EXIST = new CacheResult<>(CacheResult.CacheStatus.MAY_EXIST, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add also mapExist()
method. The is the Collections.emptyList() trick.
static <T> CacheResult<T> mapExist() {
return CacheResult<T> MAY_EXIST;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thanks for the patch @sumitagrawl
What changes were proposed in this pull request?
can create TypedTable with custom key and value Codec, this will be used for customized codec where it may be required to avoid some field for performance.
Also added TABLE_NO_CACHE type caching, so that TypedTable becomes light-weight in creation and operation; and complexity / threads present PARTIAL_TABLE_CACHE class can be avoided.
Sample:
1.
Based on different codec and usecase, performance can be achieved,
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-12582
How was this patch tested?