[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-12-23 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
@jbertram my concerns have been addressed


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-12-20 Thread jbertram
Github user jbertram commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
@clebertsuconic, @michaelandrepearce, do you guys think this is ready to be 
merged now? It looks good to me since there is a test and the file is always 
synced on `releaseResources`.


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
Re testing could you simply not use a mocking framework to assert that 
file.sync was invoked when releaseResources is called? This way would ensure 
the file sync change isnt regressed from there.


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
I would just always sync on releaseResources. I don't see any case where we 
wanted to close without a previous sync given the semantic I missed.


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread wy96f
Github user wy96f commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
> nice catch then.. i will merge this and back port into 2.6.x

I just found in ServerSessionImpl::messageToLargeMessage, 
StompSession::sendInternalLarge
, ReplicationEndpoint::stop we need a sync too.

Could we add a sync parameter in releaseResources? If true, file will be 
synced before closing.


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
nice catch then.. i will merge this and back port into 2.6.x


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread wy96f
Github user wy96f commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
> I am a bit confused as a file.close would issue a sync.

It won't. As man page said:
A successful close does not guarantee that the data has been successfully 
saved to disk, as the kernel defers writes.  It is not common for a file system 
to flush the buffers when the stream is closed.  If you need to be sure that 
the data is physically stored use fsync(2).  (It will depend on the disk 
hardware at this point.)



---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
The release resources thing. 


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread clebertsuconic
Github user clebertsuconic commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
I am a bit confused as a file.close would issue a sync.  


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread wy96f
Github user wy96f commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
> From the description it is unclear what is meant by "server crash". JVM 
crash or host/machine crash. If the problem occured with unexpected JVM process 
termination maybe the broker can be started in a separate JVM processes and 
SIGKILLed while doing large message processing to reproduce the issue.

In our case it's a host crash. I don't think jvm crash or sending SIGKILL 
can reproduce it as franz1981 said linux os would take care to flush the page 
cache.


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread sebthom
Github user sebthom commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
From the description it is unclear what is meant by "server crash". JVM 
crash or host/machine crash. If the problem occured with unexpected JVM process 
termination maybe the broker can be started in a separate JVM processes and 
SIGKILLed while doing large message processing to reproduce the issue.


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
I appreciate its not easy to replicate. But with some mocks maybe it might. 
The key thing is how do we ensure this is never regressed? 


---


[GitHub] activemq-artemis issue #2444: ARTEMIS-2186 Large message incomplete when ser...

2018-11-27 Thread franz1981
Github user franz1981 commented on the issue:

https://github.com/apache/activemq-artemis/pull/2444
  
A test would not easily hit this: most OS take care to flush the page cache 
if a process crash "fsyncing" any pending contents. Only using a VM on a real 
machine shutdown would emulate this behaviour.


---