Skip to content
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

Error when the difference between test_by_score and tests rows is > 0 and you try to run node server #32

Open
brando90 opened this issue Mar 25, 2014 · 6 comments
Labels

Comments

@brando90
Copy link
Collaborator

Currently I was trying to run node server and the following error comes up:

events.js:72
        throw er; // Unhandled 'error' event
              ^
TypeError: Cannot read property 'hash' of undefined
    at CassandraBackend.initTestPQ (/home/brandomiranda/Documents/6.S194/testreduce/CassandraBackend.js:154:44)
    at CassandraBackend.queryCB (/home/brandomiranda/Documents/6.S194/testreduce/CassandraBackend.js:135:17)
    at Object.callback (/home/brandomiranda/Documents/6.S194/testreduce/node_modules/node-cassandra-cql/index.js:248:16)
    at Connection.handleFrame (/home/brandomiranda/Documents/6.S194/testreduce/node_modules/node-cassandra-cql/lib/connection.js:244:15)
    at FrameParser.<anonymous> (/home/brandomiranda/Documents/6.S194/testreduce/node_modules/node-cassandra-cql/lib/connection.js:45:12)
    at FrameParser.EventEmitter.emit (events.js:92:17)
    at emitReadable_ (_stream_readable.js:408:10)
    at emitReadable (_stream_readable.js:404:5)
    at readableAddChunk (_stream_readable.js:165:9)
    at FrameParser.Readable.push (_stream_readable.js:127:10)

To summarize what that error means is that its trying to get the attribute .hash from an undefined object:

var lastCommit = this.commits[commitIndex].hash;

It happens on on the following line for me:
https://github.com/gwicke/testreduce/blob/master/CassandraBackend.js#L136

I am 99% certain that the error is triggered because the following line of code executes:

https://github.com/gwicke/testreduce/blob/master/CassandraBackend.js#L129

Which makes initTestPQ to be called again which makes indexing something out of range which returns a undefined that does not have a .hash attribute.

Line 129 is called when the difference in lines between tests and test_by_score is >0. In my case I called the usual importCassandra.js to start the database.keyspace:

https://github.com/gwicke/testreduce/blob/master/articles/importCassandra.js

and then manually inserted one commit to the database with the following command:

"INSERT INTO commits (hash, tid, keyframe) VALUES ('0b5db8b91bfdeb0a304b372dd8dda123b3fd1ab6', now(), true)"

[NOTE: I inserted the ' next to the hash because otherwise, github tries to link it to some actual commit. When writing it to cqlsh, make sure to remove the ' because it should not be there as a cql command]

When I ran importCassandra, I ran it with some languages, I don't remember exactly which ones I ran but the state of my database is the following (I probably ran it with en, ar and sv languages, since those are the only abbreviations I really know):

tests table has 18592 rows and the test_by_score table has 18591.

Their difference is > 0 and that is why the code I pointed out is even running and crashing (the reason is crashing is because commitIndex = 1, and there isn't a second commit, so indexing the array returns undefined and accessing the .hash property of undefined of course fails).

On line 129 of CassandraBackend.js the difference between numTestsLeft - results.rows.length > 0, which is why the redo initTestPQ function runs.

Not sure why the difference is positive or why it would matter. My guesses are that for each commit we want their corresponding test score, so thats why they have to have the same number of rows.

@brando90 brando90 changed the title Error when the difference between test_by_score and tests rows is > 0 Error when the difference between test_by_score and tests rows is > 0 and you try to run node server Mar 25, 2014
@jjchen
Copy link
Collaborator

jjchen commented Mar 26, 2014

Regarding your question about the initTestPQ function:
The function is initializing the priority queue of tests (ordered by their previous test scores) which is used to determine the order in which tests are given to clients. So basically, it's just reading from the test_by_score table for each test in the tests table and populating our in-memory priority queue. However, (if I'm remembering/understanding this correctly), for each commit, the test_by_score table saves only the tests whose scores have changed - therefore, to get the score for all the tests we have to go back through the previous commits until either we reach a keyframe commit, or we have a score for each test (hence the recursion if numTestsLeft - results.rows.length > 0).

The current code seems to have a bug on line 125 - isSnapshot should be isKeyframe. I think this fix would have the side effect of getting rid of your error, but I don't know why your tests and test_by_score tables tables have different numbers of rows in the first place (importCassandra looks like it should be inserting the same tests into both tables). Are you getting any errors running importCassandra? I also don't know if there should ever be a test without a corresponding test_by_score - would we ever insert into the tests table through some other mechanism (other than importCassandra)?

@brando90
Copy link
Collaborator Author

Hi @jjchen !

Well, did a couple of testing again and re-started everything and I was unable to reproduce the bug the way I described on the issue. However, I was able to reproduce it some other way once I decided to automate inserting a commit to importCassandra.js. I am not sure why adding the following lines makes the errors I posted above:

/*
var insertDummyCommit = function(dummyCommit, tid) {
    console.log("Calling insertDummyCommit");
    var queryCommit = "INSERT INTO commits (hash, tid, keyframe) VALUES (?, ?, ?)",
        commit_hash = new Buffer(DUMMYCOMMIT),
        tid = 'now()'
        keyframe = true;
    client.execute(queryCommit, [commit_hash, tid, keyframe], 1, function (err, result){
        if (err) {
            console.log(err)
        } else {
            console.log("Dummy Commit hash insertion successful.")
        }
        console.log("Done calling inserDummy Commit, success unknown.");
});
}
*/

var loadJSON = function(prefix) {
    //insertDummyCommit(DUMMYCOMMIT); Does not work, dont know why
    var i, titles = require(['./', prefix, 'wiki-10000.json'].join(''));
    console.log('importing ' + prefix + ' wiki articles from:');
    console.log(['./', prefix, 'wiki-10000.json'].join(''));
    for (i = 0; i < titles.length; i++) {
        console.log(prefix, titles[i]);
        insertTestBlob(prefix + 'wiki', titles[i]);
        insertTestByScore(prefix + 'wiki', titles[i]);
    }
    console.log('done importing ' + prefix + ' wiki articles');
};

But it doesn't matter becuase the everything seems to be working now. Going to localhost 8001 seems to be working fine and the page doesn't seem to freak out when the database is empty so I think it works now.

@brando90
Copy link
Collaborator Author

@jjchen , also, why does it need to be isKeyframe?

@brando90 brando90 added the bug label Mar 26, 2014
@brando90
Copy link
Collaborator Author

I will close the issue once we agree on how to handle isKeyframe thing

@jjchen
Copy link
Collaborator

jjchen commented Mar 26, 2014

Maybe I'm missing something, but isn't the variable name isKeyframe instead
of isSnapshot in the function above this?
On Mar 26, 2014 4:40 PM, "brando90" [email protected] wrote:

@jjchen https://github.com/jjchen , also, why does it need to be
isKeyframe?

Reply to this email directly or view it on GitHubhttps://github.com//issues/32#issuecomment-38736576
.

@swong15
Copy link
Collaborator

swong15 commented Mar 28, 2014

Sorry I've been busy with a project. I'll be sending in a pull request to fix that naming error shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants