This repository has been archived by the owner on Aug 2, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 20
Refactoring the persistence layer to be able to persist any Java Object #407
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
yojs
force-pushed
the
refactor-persistence-layer
branch
from
September 1, 2020 04:05
3221cc0
to
36755dd
Compare
Codecov Report
@@ Coverage Diff @@
## master #407 +/- ##
============================================
+ Coverage 70.09% 70.80% +0.70%
- Complexity 2287 2354 +67
============================================
Files 303 307 +4
Lines 13605 13962 +357
Branches 1133 1178 +45
============================================
+ Hits 9537 9886 +349
- Misses 3695 3702 +7
- Partials 373 374 +1 Continue to review full report at Codecov.
|
Fixes: #399 The persistence layer takes care of durably storing the RCA results (and in future decisions from deciders) and providing them when asked for it. The layer also takes care of periodic file rotations and cleaning up old DB files. Although, it wasn't intended, the _Persistence_ layer is tightly coupled to the _RCA graph_ today. If you look at the `write()` method in `Persistable` interface, you will find this signature: `<T extends ResourceFlowUnit> void write(Node<?> node, T flowUnit) throws SQLException, IOException;` So the Persistable Object knows about the graph node. Therefore, in future as the remediation system evolves and we have components, that are not nodes in the RCA-Graph, we might still want to persist their outputs and this interface can't do that. Therefore, the goal is to make the methods of Persistable generic, so that it can persist any object given to it and be able to read out any object the caller asks for. Say we have to persist this class: ```java Outer.java ________________ class Outer { int x; int y; boolean z; String name; List<T> myList; B b_obj; } ------------------------------------------------------- A.java _______ class B { int x; String y; } ``` when the above code is annotated as: ```java Outer.java ________________ // This will create a table named "Outer" @table class Outer { int x; int y; // Its not annotated as column, therefore, it will not be persisted. boolean z; String name; B b_obj; List<T> myList; // Add a column named x to Outer table. The value stored in the row for that column will be the value of x. @column int getX() { .. } @column int getY() { .. } @column String getName() { .. } // For all fields that are not [primitive types](https://docs.oracle.com/javase/6/docs/api/java/lang/Class.html#isPrimitive()), they will be persisted in their own tables as a new row. The auto-increment ID for this row will be persisted as a value of the column named "__table__B", so that we have a link to the nested element/table from this table. @column B getBObj() {..} // If the Column annotation exists for a collection type, then they will be written to a table of their own. // The name of the table is obtained by calling T.class.getSimpleName(). If all of them evaluates to the same // table name, then they get written to the same table as different rows. And the corresponding Outer table's // column will have a string [<rowId1]>, <rowId2>]. // If `T.class.getSimpleName()` evaluates to different names, then they are linked back in Outer table as // different columns. @column List<T> getMyList() { .. } } ------------------------------------------------------- B.java _______ @table class B { int x; String y; } ``` this creates these set of tables: Here assuming that `getMyList()` returns a list of three elements where ```java for (T in getMyList) { // T.getClass().getSimpleName() returns `T` for two of the objects and `TT` for the third. } ``` Table Outer timestamp|ID|X|Y|Name|__table__BObj|__table__T|__table__TT --|--|--|-|------|--------------|----------------|--- ..|53|1|2|name|34|[23,24]|32 Table B timestamp|ID|X|Y --|--|--|-- ..|34|23|y Table T timestamp|ID| col2| .. --|---|-|-------- ..|23|24|.. ..|24|24|.. Table TT timestamp|ID| col2| .. --|---|-|-------- ..|32|24|.. - The Java code resembles how the tables and nested tables will be laid out. - Because the nested table can be obtained just by reading the column name, someone can write a tool to read the RCAs from the SQLite files. The won't need the java package to figure out the nesting. - If new columns or nestings are added, they can also be added in the DB without schema mismatch. This is because, we create a new SQLite file on each restart of the RCA agent process (on top of periodic file rotations). - The table name and column names are derived from the classname and getter name (methodName with `get` stripped out). This provides a 1-1 mapping from the persistor class and fields to the table name and column names. - Over and above, we are able to persist any Java Object.
yojs
force-pushed
the
refactor-persistence-layer
branch
from
September 1, 2020 18:06
36755dd
to
edc7205
Compare
yojs
added
the
code-refactoring
The change reduces the cognitive load of the reader of the code and makes adding new changes easier
label
Sep 1, 2020
yojs
changed the title
First take - persist any Object into database not just FlowUnits
Refactoring the persistence layer to be able to persist any Java Object
Sep 1, 2020
sruti1312
previously approved these changes
Sep 1, 2020
...com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SQLitePersistor.java
Outdated
Show resolved
Hide resolved
...com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SQLitePersistor.java
Show resolved
Hide resolved
...com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SQLitePersistor.java
Outdated
Show resolved
Hide resolved
...com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SQLitePersistor.java
Show resolved
Hide resolved
...com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SQLitePersistor.java
Show resolved
Hide resolved
...azon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SqliteObjectPersistor.java
Outdated
Show resolved
Hide resolved
sruti1312
previously approved these changes
Sep 2, 2020
rguo-aws
reviewed
Sep 2, 2020
...com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SQLitePersistor.java
Show resolved
Hide resolved
...com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/persistence/SQLitePersistor.java
Show resolved
Hide resolved
rguo-aws
approved these changes
Sep 2, 2020
sruti1312
approved these changes
Sep 2, 2020
The failed test is |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
code-refactoring
The change reduces the cognitive load of the reader of the code and makes adding new changes easier
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.
Fixes #: #399
Description of changes:
The persistence layer takes care of durably storing the RCA results (and in future decisions from deciders) and providing them when asked for it. The layer also takes care of periodic file rotations and cleaning up old DB files.
Although, it wasn't intended, the Persistence layer is tightly coupled with the RCA graph today. If you look at the
write()
method inPersistable
interface, you will find this signature:<T extends ResourceFlowUnit> void write(Node<?> node, T flowUnit) throws SQLException, IOException;
So the
Persistable
Object knows about the graph node. Therefore, in future as the remediation system evolves and we have components, that are not nodes in the RCA-Graph, we might still want to persist their outputs. But this interface can't do that.Therefore, the goal is to make the methods of Persistable generic, so that it can persist any object given to it and be able to read out any object the caller asks for.
Say we have to persist this class:
when the above code is annotated as:
this creates these set of tables:
Here assuming that
getMyList()
returns a list of three elements whereTable Outer
Table B
Table T
Table TT
What this buys us
get
stripped out). This provides a 1-1 mapping from the persistor class and fields to the table name and column names.Tests:
Added unit tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.