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

ISPN-7530 InfinispanStoreRocksDBIT leaves files in the current directory #4902

Conversation

danberindei
Copy link
Member

</property>
<!-- To debug the Arquillian managed application server: -->
<!-- property name="javaVmArguments">-Xrunjdwp:transport=dt_socket,address=8000,server=y,suspend=y -Xmx512m -Dorg.jboss.remoting-jmx.timeout=300 -Djava.net.preferIPv4Stack=true -Djgroups.bind_addr=127.0.0.1</property-->
<!--<property name="javaVmArguments">-Xrunjdwp:transport=dt_socket,address=8000,server=n,suspend=y -Dorg.jboss.remoting-jmx.timeout=300-->
<!-- -Djboss.socket.binding.port-offset=100 -Xmx512m -Djava.net.preferIPv4Stack=true-->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this file could be probably removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really want to keep this in because the existing commented javaVmArguments were incomplete, and it was frustrating when I enabled debugging and the test wouldn't run.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, could you please add a comment with those switches at the beginning of the file? Let's now throw rubbish all around :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slaskawi I could remove the comment altogether, but I would definitely not leave the comment as it was before...

builder.persistence()
.addStore(RocksDBStoreConfigurationBuilder.class)
.location(tmpDirectory)
.expiredLocation(tmpDirectory + "-expired");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one! :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused... shouldn't we use expireDir instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think RocksDB needs both those directories (but I might be wrong). Maybe @tristantarrant would know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok but here you are using

tmpDirectory(this.getClass()) + "-expired"

and in the beginning you are creating

private static String expiredDir = tmpDirectory(InfinispanStoreRocksDBIT.class) + File.separator + "rocksdb-expiredtestcache";

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed @pruivo

@danberindei danberindei force-pushed the ISPN-7530_InfinispanStoreRocksDBIT_cleanup branch from 3d5e190 to 3ab4b24 Compare February 27, 2017 16:57
@danberindei
Copy link
Member Author

Updated to fix RocksDBCacheStoreIT, I missed the fact that dataDir had to be in sync with the XML configuration in rocksdb-(dist|repl|local).xml.

@danberindei danberindei force-pushed the ISPN-7530_InfinispanStoreRocksDBIT_cleanup branch 2 times, most recently from 75c04ec to 646b08b Compare March 1, 2017 12:49
@pruivo pruivo added the Question label Mar 1, 2017
@danberindei danberindei force-pushed the ISPN-7530_InfinispanStoreRocksDBIT_cleanup branch from 646b08b to bcc5778 Compare March 2, 2017 10:13
</property>
<!-- To debug the Arquillian managed application server: -->
<!-- property name="javaVmArguments">-Xrunjdwp:transport=dt_socket,address=8000,server=y,suspend=y -Xmx512m -Dorg.jboss.remoting-jmx.timeout=300 -Djava.net.preferIPv4Stack=true -Djgroups.bind_addr=127.0.0.1</property-->
<!--<property name="javaVmArguments">-Xrunjdwp:transport=dt_socket,address=8000,server=n,suspend=y -Dorg.jboss.remoting-jmx.timeout=300-->
<!-- -Djboss.socket.binding.port-offset=100 -Xmx512m -Djava.net.preferIPv4Stack=true-->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, could you please add a comment with those switches at the beginning of the file? Let's now throw rubbish all around :D

builder.persistence()
.addStore(RocksDBStoreConfigurationBuilder.class)
.location(tmpDirectory)
.expiredLocation(tmpDirectory + "-expired");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think RocksDB needs both those directories (but I might be wrong). Maybe @tristantarrant would know.

@danberindei danberindei force-pushed the ISPN-7530_InfinispanStoreRocksDBIT_cleanup branch 2 times, most recently from dd278d7 to 65c485e Compare March 6, 2017 13:43
@pruivo
Copy link
Member

pruivo commented Mar 6, 2017

@danberindei and @slaskawi 👍 looks good to me.

@danberindei danberindei force-pushed the ISPN-7530_InfinispanStoreRocksDBIT_cleanup branch from 65c485e to f88ed8e Compare March 7, 2017 10:08
@danberindei danberindei force-pushed the ISPN-7530_InfinispanStoreRocksDBIT_cleanup branch from f88ed8e to a93972a Compare March 7, 2017 15:17
@pruivo pruivo merged commit e28b8c2 into infinispan:master Mar 8, 2017
@danberindei danberindei deleted the ISPN-7530_InfinispanStoreRocksDBIT_cleanup branch March 9, 2017 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants