deanhiller / playorm

ORM for nosql with SCALABLE SQL
https://github.com/deanhiller/playorm/wiki
Other
76 stars 18 forks source link

NPE on index pointing to nonexistent row #116

Closed hsn10 closed 11 years ago

hsn10 commented 11 years ago

i deleted some rows manually from other program while index entry still exists and it leads to NPE. There is really need for reindex tool.

java.lang.NullPointerException at com.alvazan.orm.layer0.base.BaseEntityManagerImpl.remove(BaseEntityMa nagerImpl.java:320) at actors.ReadCounter$$anonfun$receive$1.applyOrElse(ReadCounter.scala:2 6) at akka.actor.ActorCell.receiveMessage(ActorCell.scala:425)

easility commented 11 years ago

There is one command REINDEX for the same. Did you tried that? Read http://buffalosw.com/wiki/How-to-run-the-re-indexing-on-tables/

hsn10 commented 11 years ago

Real problem is that it.next().getValue() returns null if index points to invalid row, should not it delete invalid index and skip to next row?

       val it = q.getResultsIter(false).iterator()
       while (it.hasNext()) {
         val t = it.next().getValue()
deanhiller commented 11 years ago

In your example, it.next().getKey will return the key in the index that has no value and it.next.getValue will return null. This is so you can log index corruption. We didn't necessarily want clients to not know about the corruption and completely hide it from them which is why we return a KeyValue object which has the key from the index and the value is the row if it exists. I didn't really know another way to communicate to the client a corrupt index. If you don't care, you can always just skip the null entries which we do in alot of cases where we don't care but in other cases we do care and trigger an update of things. Should we close this or perhaps, there should be a special iterator that returns the entity(ie. you would no longer need to call getValue() in that case)

hsn10 commented 11 years ago

There are 2 problems even after REINDEX. I start with empty table, no index entries. I insert one row, it correctly insert inself to index.

now there are 2 problems.

i run loop:

   val em = NoSql.em()
   val q = em.createNamedQuery(classOf[NoteRead], "getall")
     var cleaned = true
     val uid = new OpenHashMap[String,Any](100)
     while ( cleaned ) {
       cleaned = false
       val it = q.getResultsIter(false).iterator()
       while (it.hasNext()) {
         cleaned = true
         val t = it.next().getValue()
         println(t)
         // remove record from read log
         em.remove(t)

1 it returns row and in next iteration null from getValue() which means that it.hasNext() is broken.

2 It calls em.remove(t) on first now and in second iteration it throws NPE due to it.hasNext problem; note flush() is not called.

row is removed from table but index entry is still there, now pointing to nonexistant row.

hsn10 commented 11 years ago

I added debug prints to see what is going on in more detail:

[debug] application - running query 7727963068700607134:10.0.0.1:Mozilla/5.0 (Windows NT 5.1 1 Firefox/22.0 [debug] application - flushing em [debug] application - running query null [ERROR] [07/16/2013 16:45:18.309] [application-akka.acto [akka://application/user/readcounter] null java.lang.NullPointerException

it means that in that 1 row example, flush() is called but row is deleted while index is not. In second pass query scans index it points to nonexistent row and getValue() returns null.

hsn10 commented 11 years ago

here is data model:

@NoSqlQuery(name="getall", query="SELECT * FROM TABLE") @NoSqlEntity class NoteRead(what:Long, whom:String) {

@NoSqlId @NoSqlIndexed private var id:Long = Random.nextLong()

private var note:Long = what }

deanhiller commented 11 years ago

Vikas, can you write a test case on this and debug this issue. (I am also wondering why we haven't seen this as we have been in production a while now but maybe our use case is slightly different or something).

thanks, Dean

hsn10 commented 11 years ago

I am running same loop with different entity, and this works fine (index entry is removed)

@NoSqlEntity @NoSqlQuery(name="getexpired", query="SELECT * FROM TABLE as t WHERE t.created < :time") class Token { /* Unique random Token Id / @NoSqlId private var id:Long = 0

/* when was this token created. This timestamp is used for cleaning old tokens. This timestamp will be refreshed on post using this token to get extended lifetime before token is cleaned from db. / @NoSqlIndexed private var created:Long = 0

/* what note was created using this token. Used for redirect after post with used token. / private var post:Long = 0 }

hsn10 commented 11 years ago

I got it. It does not delete index entry if i index Id field

@NoSqlId @NoSqlIndexed

because in my case any index is good enough because i am selecting all rows, i switched index to field long note and everything works fine as expected.

hsn10 commented 11 years ago

Vikas, are you able to reproduce this problem? its high priority for me.

deanhiller commented 11 years ago

I saw an email on this from him so I think he was able to....he is asleep right now though I believe as he is in a totally different timezone.

easility commented 11 years ago

We tried it and unfortunately we are not able to reproduce it. We have our entity as

    @NoSqlEntity
    @NoSqlQueries({
@NoSqlQuery(name="findByName", query="select u from TABLE as u")
})
     public class Test {

@NoSqlId
@NoSqlIndexed
private String id;

@NoSqlIndexed
private String name;

private long numTimes;
 }

and then we created 3 rows(using Playorm)..and then delete one row (from Cassandra CLI)..we checked using CLI that there are still some values in our indices table for the deleted entry..

then execute following test method:

 private static void test(NoSqlEntityManagerFactory factory) {
    NoSqlEntityManager em = factory.createEntityManager();
    Query<Test> q = em.createNamedQuery(Test.class, "findByName");
    Iterable<KeyValue<Test>> list  = q.getResultsIter();
    Iterator<KeyValue<Test>> it = list.iterator();
    while (it.hasNext()) {
        Test s = it.next().getValue();
        System.out.println("Test Id " + s.getId());
        System.out.println("TEST name " + s.getName());
        System.out.println("Test numTimes  " + s.getNumTimes());
    }
}

and it successfully returned 2 rows..

hsn10 commented 11 years ago

try remove @NoSqlIndexed from name

easility commented 11 years ago

We did that first and later add it..which I pasted here..

hsn10 commented 11 years ago

i am writing my own test

hsn10 commented 11 years ago

You are right, in simple test - new EntityManager is created before test, it works as expected. I had more complicated code, which did deletetions and reads from same CF.

hsn10 commented 11 years ago

i checkouted my old source from git and now i can reproduce this every time. If i fail to rework it into small example, are you willing to sign NDA?

hsn10 commented 11 years ago

Problem is only in playorm 1.6, in playorm 1.7-SNAPSHOT it didnt throw NPE. #123 still exists in 1.7-SNAPSHOT

hsn10 commented 11 years ago

code for reproducing is here: https://github.com/hsn10/ormtest/tree/npeonbadindex

easility commented 11 years ago

I have added a check..and the error is not coming now..can you please check on latest code?

hsn10 commented 11 years ago

latest code passes test

easility commented 11 years ago

Great. Closing this defect.