Added files related for backup test #56
Conversation
There was a problem hiding this comment.
Overall, it feels like the automated testing code wasn't a good fit for going into the ingestion service itself. It is also awkward to use junit as an automated/integration test framework. Instead the concepts in CreateInfluxDBTests and the entry point RestoreDBInfluxDB should really just go into a new, standalone application.
Even more generally, I can't tell how the execution of the code fits into the overall testing process especially since configuration currently involves literal strings that require code changes. Ultimately, https://github.com/racker/ceres-ingestion-service/blob/master/disaster_recovery.md should be updated as part of the changes to explain how someone else would run this backup validation after using the restore process.
| @Test | ||
| public void isDBExistinRestore(){ | ||
| // CreateInfluxDB influxDBConnection1 = new CreateInfluxDB("","","","",""); | ||
| CreateInfluxDB influxDBConnection1 = new CreateInfluxDB(username,password,openurl,database,retentionPolicy); |
There was a problem hiding this comment.
influxDBConnection1 doesn't seem to be used and should be removed to avoid confusion
| QueryResult result = new QueryResult(); | ||
| result.setResults(resultList); | ||
| Mockito.when(influxDB.query(Mockito.any())).thenReturn(result); | ||
| assertTrue(result.getResults().size() == 1); |
There was a problem hiding this comment.
This seems to only assert the result that was mocked in the line before it, so there doesn't seem to be anything important tested in isDBExistinRestore.
| QueryResult result = new QueryResult(); | ||
| result.setResults(resultList); | ||
| Mockito.when(influxDB.query(Mockito.any())).thenReturn(result); | ||
| assertTrue(result.getResults().size() == 0); |
There was a problem hiding this comment.
Same for this. It's only asserting what you told the mock to return.
| // Copy all measurements from db_0 to Db_1 restore database. | ||
| QueryResult copyDb = influxDB.query(new Query("SELECT * INTO db_1.autogen.:MEASUREMENT FROM db_0.rp_5d./.*/ GROUP BY *", "db_0")); | ||
| // System.out.println("Copy of all databases, measurements and datas has been restored"); | ||
|
|
||
| // Compare Backup and the Restore DB |
There was a problem hiding this comment.
From the comments and from what I can tell of the SELECT * INTO ... this copies db_0 into db_1 and then compares the two? Isn't that always going to pass?
There was a problem hiding this comment.
@hravi76 db_0 is not a backup db. Jira description also states:"Ingestion service collects and caches incoming data in-memory. After a particular amount of time, that data is dumped into a file into a GCP bucket (one of the backup buckets based on Application.yml configuration)." Then database should be restored from a backup file.
Please follow tests instruction from here https://github.com/racker/ceres-ingestion-service/blob/master/disaster_recovery.md
| import java.util.List; | ||
| import java.util.concurrent.TimeUnit; | ||
|
|
||
| public class NotcreatedDBInfluxDB { |
There was a problem hiding this comment.
This class name is confusing. At least some javadoc could help explain why it is "not created DB", such as:
/**
This class ...
*/
| // Password | ||
| private String password; | ||
| // Connection address | ||
| private String openurl; |
There was a problem hiding this comment.
What does the "open" part of openurl refer to? Perhaps call it "influxDbUrl" or "instanceUrl"
| // @SuppressWarnings("deprecation") | ||
| public void createDB(String dbName) throws IOException { | ||
| influxDB.createDatabase(dbName); | ||
| System.out.println("Influxdb connection successful"); | ||
| } | ||
| */ |
There was a problem hiding this comment.
Remove dead code
There are several others places where you have chunks of commented out code that should be removed or cleaned up.
| // Show measurements in restore Db db_1 | ||
|
|
||
| int restblecount = 0; | ||
| Query query = new Query("SHOW MEASUREMENTS", "db_1"); | ||
| List<QueryResult.Result> restoredbresults = influxDB.query(query).getResults(); | ||
| List<List<Object>> restoredbtble = restoredbresults.get(0).getSeries().get(0).getValues(); | ||
| // System.out.println("Tables in Restore Db db_1 ----" + restoredbtble); | ||
|
|
||
| for (int i = 0; i < restoredbtble.size(); i++) { | ||
| List<Object> restoretble = restoredbtble.get(i); | ||
| for (Object restoretable : restoretble) { | ||
| //QueryResult rowcount = influxDB.query(new Query("SELECT count(*) FROM /.*/" , "db_1")); | ||
| QueryResult rowcount = influxDB.query(new Query("SELECT count(*) FROM "+ restoretable , "db_1")); | ||
| List<QueryResult.Result> restoretbleresults =rowcount.getResults(); | ||
| if(restoretbleresults.get(0).hasError()) { | ||
| }else{ | ||
| List<List<Object>> resdbtblecnt = restoretbleresults.get(0).getSeries().get(0).getValues(); | ||
| // System.out.println("RowCount in restore db table " + resdbtblecnt); | ||
|
|
||
| // Get the row count of each table from the restore database | ||
|
|
||
| for (int j = 0; j < resdbtblecnt.size(); j++) { | ||
| List<Object> resrowcount = resdbtblecnt.get(j); | ||
| // System.out.println("Row count from each table in restore db " + resrowcount.get(1)); | ||
| resrowcountval = Float.valueOf(resrowcount.get(1).toString()); | ||
| resrwcnt.add(Float.valueOf(resrowcount.get(1).toString())); | ||
| //System.out.println("Row count from each table in restore db " + resrowcountval); | ||
|
|
||
| } | ||
|
|
||
| } | ||
| restblecount = i+1; | ||
| } | ||
| } | ||
|
|
||
| // System.out.println("Total Table in restore db " + restblecount); | ||
|
|
||
| // Compare total db count in backup db and restore db | ||
|
|
||
| if(bkdbcount == resdbcount) { | ||
| // System.out.println("DB count equal in Backup and Restore DB"); | ||
| }else | ||
| { | ||
| // System.out.println("DB count not equal in Backup and Restore DB"); | ||
| } | ||
|
|
||
| // Compare total table count in backup db and restore db | ||
|
|
||
| if(bktblecount == restblecount) { | ||
| // System.out.println("Table count equal in Backup and Restore DB"); | ||
| }else | ||
| { | ||
| // System.out.println("Table count not equal in Backup and Restore DB"); | ||
| } | ||
|
|
||
| // Compare total rowcount in backup db and restore db | ||
| if(bkrwcnt.equals(resrwcnt)){ | ||
| // System.out.println("Rowcount in all the tables are equal!!!!"); | ||
| } | ||
| else{ | ||
| // System.out.println("Rowcount are not correct!!!!"); | ||
| } | ||
|
|
||
| } | ||
| catch (Exception e){ | ||
| if(e.equals(null)) { | ||
| System.out.println("False"); | ||
| } | ||
| e.printStackTrace(); | ||
| } |
There was a problem hiding this comment.
I can't tell what the benefit is for all this code. The println statements are all commented out and even then an automated verification shouldn't rely on printed output, but instead should programmatically compare and raise exceptions or log errors if comparisons fail. It could also conclude with a non-zero System.exit(...) call so that CI/CD etc tooling can detect the failure in the test execution.
| QueryResult queryResult = influxDB.query(dbquery); | ||
| List<QueryResult.Result> bkdbresults = queryResult.getResults(); | ||
| List<List<Object>> bkfinaldbresults = bkdbresults.get(0).getSeries().get(0).getValues(); | ||
| assertTrue(bkfinaldbresults.size() == 5); |
There was a problem hiding this comment.
Where does "5" come from? I don't see any canned test data as part of this PR, so I'm not sure how the unit test can make such a specific assertion.
| } | ||
| } | ||
| } | ||
| assertTrue(bkrwcnt.equals(resrwcnt)); |
There was a problem hiding this comment.
As mentioned in the comment in NotcreatedDBInfluxDB, db_0 was copied over to db_1 so it seems like this assertion will always succeed.
| // System.out.println("Row count from each table in restore db " + resrowcount.get(1)); | ||
| resrowcountval = Float.valueOf(resrowcount.get(1).toString()); | ||
| resrwcnt.add(Float.valueOf(resrowcount.get(1).toString())); | ||
| //System.out.println("Row count from each table in restore db " + resrowcountval); |
There was a problem hiding this comment.
@hravi76 code from line 187-201 is repeating code on lines 152-177. Please create a helper method to return row count.
| } | ||
| assertTrue(bkrwcnt.equals(resrwcnt)); | ||
|
|
||
| if(bkrwcnt.equals(resrwcnt)){ |
There was a problem hiding this comment.
@hravi76 error message should go into assert statement, so it will be printed if assert has failed
Resolves
JIRA - 277
What
Creation of restore DB. Copying the Measurements and datas to restore DB. Comparing the results.
How to test
Why
<Why was the final approach taken? Were alternatives considered?>
TODO
<outstanding tasks before this PR is considered 'ready'>