You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2017/09/07 23:11:45 UTC
Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/
-----------------------------------------------------------
Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
Diffs
-----
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
Diff: https://reviews.apache.org/r/62179/diff/1/
Testing
-------
Thanks,
Jinmei Liao
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jinmei Liao <ji...@pivotal.io>.
> On Sept. 7, 2017, 11:34 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
> > Lines 107 (patched)
> > <https://reviews.apache.org/r/62179/diff/1/?file=1818168#file1818168line109>
> >
> > How do you think this reads this instead of the `for` loop?
> > ```
> > Consumer<MemberVM> stopMemberAndDisconnectDS = (MemberVM memberVM) -> {
> > memberVM.stopMember();
> > memberVM.getVM().invoke((SerializableRunnableIF) MemberStarterRule::disconnectDSIfAny);
> > };
> >
> > Arrays.stream(members).filter(Objects::nonNull).forEach(stopMemberAndDisconnectDS);
> > ```
this way, we are still only calling disconnectDSIfAny() when the members element is not null. When a VM is used only as a client, the members[] won't contain that VM at all, because it only contains the ServerVM and LocatorVM.
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/#review184915
-----------------------------------------------------------
On Sept. 7, 2017, 11:12 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 11:12 p.m.)
>
>
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
>
>
> Diff: https://reviews.apache.org/r/62179/diff/1/
>
>
> Testing
> -------
>
> precheckin running
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/#review184915
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
Lines 107 (patched)
<https://reviews.apache.org/r/62179/#comment261114>
How do you think this reads this instead of the `for` loop?
```
Consumer<MemberVM> stopMemberAndDisconnectDS = (MemberVM memberVM) -> {
memberVM.stopMember();
memberVM.getVM().invoke((SerializableRunnableIF) MemberStarterRule::disconnectDSIfAny);
};
Arrays.stream(members).filter(Objects::nonNull).forEach(stopMemberAndDisconnectDS);
```
- Jared Stewart
On Sept. 7, 2017, 11:12 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 11:12 p.m.)
>
>
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
>
>
> Diff: https://reviews.apache.org/r/62179/diff/1/
>
>
> Testing
> -------
>
> precheckin running
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jinmei Liao <ji...@pivotal.io>.
> On Sept. 8, 2017, 4:37 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
> > Lines 232 (patched)
> > <https://reviews.apache.org/r/62179/diff/1-2/?file=1818168#file1818168line243>
> >
> > Do you think there is any chance someone will start both a server and a client in the same VM? (To deal with that I think we would just remove the `else{` and always try to disconnect the DS, even if `member` was not `null`)
the member.stopMember() eventually calls MemberStarerRule.stopMember() which will call disconnectDS already. Is it still necessary to this?
- Jinmei
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/#review184984
-----------------------------------------------------------
On Sept. 8, 2017, 3:51 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 3:51 p.m.)
>
>
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsShareConfigurationDUnitTest.java 1a5fc3485e159ca311247e617d5bec2b37c6ee10
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java 3e9227e99ad57624b024498d0ff5e807c8853221
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java 31c58177d60c9cf9ad0d07fc7a7daf8b332d3c9e
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java b35270e2d97930cee68d8c54221a04c20dfb96de
> geode-wan/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationIndexWithFromClauseDUnitTest.java c301d587c04651a03170e3da6451ebf2acf063c0
>
>
> Diff: https://reviews.apache.org/r/62179/diff/2/
>
>
> Testing
> -------
>
> precheckin running
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jared Stewart <js...@pivotal.io>.
> On Sept. 8, 2017, 4:37 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
> > Lines 232 (patched)
> > <https://reviews.apache.org/r/62179/diff/1-2/?file=1818168#file1818168line243>
> >
> > Do you think there is any chance someone will start both a server and a client in the same VM? (To deal with that I think we would just remove the `else{` and always try to disconnect the DS, even if `member` was not `null`)
>
> Jinmei Liao wrote:
> the member.stopMember() eventually calls MemberStarerRule.stopMember() which will call disconnectDS already. Is it still necessary to this?
Hm, maybe I'm missing something. All I see in MemberStarterRule.stopMember() is ` abstract void stopMember();`, and it doesn't look like either of the concrete implementations (in LocatorStarterRule and ServerStarterRule) explicitly disconnect the DS. Perhaps `InternalLocator.stop()` and `CacheServer.stop()` already disconnect the DS? If that's the case I don't think it should be necessary for us to disconnect it again.
- Jared
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/#review184984
-----------------------------------------------------------
On Sept. 8, 2017, 3:51 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 3:51 p.m.)
>
>
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsShareConfigurationDUnitTest.java 1a5fc3485e159ca311247e617d5bec2b37c6ee10
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java 3e9227e99ad57624b024498d0ff5e807c8853221
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java 31c58177d60c9cf9ad0d07fc7a7daf8b332d3c9e
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java b35270e2d97930cee68d8c54221a04c20dfb96de
> geode-wan/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationIndexWithFromClauseDUnitTest.java c301d587c04651a03170e3da6451ebf2acf063c0
>
>
> Diff: https://reviews.apache.org/r/62179/diff/2/
>
>
> Testing
> -------
>
> precheckin running
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Ken Howe <kh...@pivotal.io>.
> On Sept. 8, 2017, 4:37 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
> > Line 107 (original), 107 (patched)
> > <https://reviews.apache.org/r/62179/diff/1-2/?file=1818168#file1818168line109>
> >
> > I like this use of `IntStream`!
+1 Much clearer to read
> On Sept. 8, 2017, 4:37 p.m., Jared Stewart wrote:
> > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
> > Lines 232 (patched)
> > <https://reviews.apache.org/r/62179/diff/1-2/?file=1818168#file1818168line243>
> >
> > Do you think there is any chance someone will start both a server and a client in the same VM? (To deal with that I think we would just remove the `else{` and always try to disconnect the DS, even if `member` was not `null`)
>
> Jinmei Liao wrote:
> the member.stopMember() eventually calls MemberStarerRule.stopMember() which will call disconnectDS already. Is it still necessary to this?
>
> Jared Stewart wrote:
> Hm, maybe I'm missing something. All I see in MemberStarterRule.stopMember() is ` abstract void stopMember();`, and it doesn't look like either of the concrete implementations (in LocatorStarterRule and ServerStarterRule) explicitly disconnect the DS. Perhaps `InternalLocator.stop()` and `CacheServer.stop()` already disconnect the DS? If that's the case I don't think it should be necessary for us to disconnect it again.
I agree with Jinmei, looks to me that MemberStarterRule.stopMember() eventually calls MemberStarterRule.after(), which will then disconnectDSIfAny
- Ken
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/#review184984
-----------------------------------------------------------
On Sept. 8, 2017, 3:51 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 3:51 p.m.)
>
>
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsShareConfigurationDUnitTest.java 1a5fc3485e159ca311247e617d5bec2b37c6ee10
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java 3e9227e99ad57624b024498d0ff5e807c8853221
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java 31c58177d60c9cf9ad0d07fc7a7daf8b332d3c9e
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java b35270e2d97930cee68d8c54221a04c20dfb96de
> geode-wan/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationIndexWithFromClauseDUnitTest.java c301d587c04651a03170e3da6451ebf2acf063c0
>
>
> Diff: https://reviews.apache.org/r/62179/diff/2/
>
>
> Testing
> -------
>
> precheckin running
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/#review184984
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
Line 107 (original), 107 (patched)
<https://reviews.apache.org/r/62179/#comment261210>
I like this use of `IntStream`!
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java
Lines 232 (patched)
<https://reviews.apache.org/r/62179/#comment261209>
Do you think there is any chance someone will start both a server and a client in the same VM? (To deal with that I think we would just remove the `else{` and always try to disconnect the DS, even if `member` was not `null`)
- Jared Stewart
On Sept. 8, 2017, 3:51 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 3:51 p.m.)
>
>
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsShareConfigurationDUnitTest.java 1a5fc3485e159ca311247e617d5bec2b37c6ee10
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java 3e9227e99ad57624b024498d0ff5e807c8853221
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java 31c58177d60c9cf9ad0d07fc7a7daf8b332d3c9e
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java b35270e2d97930cee68d8c54221a04c20dfb96de
> geode-wan/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationIndexWithFromClauseDUnitTest.java c301d587c04651a03170e3da6451ebf2acf063c0
>
>
> Diff: https://reviews.apache.org/r/62179/diff/2/
>
>
> Testing
> -------
>
> precheckin running
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/#review184990
-----------------------------------------------------------
Ship it!
Ship It!
- Jared Stewart
On Sept. 8, 2017, 3:51 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62179/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 3:51 p.m.)
>
>
> Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
>
>
> Diffs
> -----
>
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsShareConfigurationDUnitTest.java 1a5fc3485e159ca311247e617d5bec2b37c6ee10
> geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java 3e9227e99ad57624b024498d0ff5e807c8853221
> geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java 31c58177d60c9cf9ad0d07fc7a7daf8b332d3c9e
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
> geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
> geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java b35270e2d97930cee68d8c54221a04c20dfb96de
> geode-wan/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationIndexWithFromClauseDUnitTest.java c301d587c04651a03170e3da6451ebf2acf063c0
>
>
> Diff: https://reviews.apache.org/r/62179/diff/2/
>
>
> Testing
> -------
>
> precheckin running
>
>
> Thanks,
>
> Jinmei Liao
>
>
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/
-----------------------------------------------------------
(Updated Sept. 8, 2017, 3:51 p.m.)
Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
Changes
-------
reworked stopVM in the rule to consider the client VM case. Get rid of the for loop for streaming.
Repository: geode
Description
-------
Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
Diffs (updated)
-----
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/IndexCommandsShareConfigurationDUnitTest.java 1a5fc3485e159ca311247e617d5bec2b37c6ee10
geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ShowMissingDiskStoresDUnitTest.java 3e9227e99ad57624b024498d0ff5e807c8853221
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ImportClusterConfigDistributedTest.java 31c58177d60c9cf9ad0d07fc7a7daf8b332d3c9e
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
geode-core/src/test/java/org/apache/geode/test/dunit/standalone/DUnitLauncher.java b35270e2d97930cee68d8c54221a04c20dfb96de
geode-wan/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigurationIndexWithFromClauseDUnitTest.java c301d587c04651a03170e3da6451ebf2acf063c0
Diff: https://reviews.apache.org/r/62179/diff/2/
Changes: https://reviews.apache.org/r/62179/diff/1-2/
Testing
-------
precheckin running
Thanks,
Jinmei Liao
Re: Review Request 62179: Test Rule Fix: clean up client DS when using
LocatorServerStartupRule
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62179/
-----------------------------------------------------------
(Updated Sept. 7, 2017, 11:12 p.m.)
Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick Rhomberg.
Repository: geode
Description
-------
Test Rule Fix: clean up clientVM if using LocatorServerStartUpRule to get the ClientVM
Diffs
-----
geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java f0385e21f708d9a085e7129d82fb3101e2fb8322
geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java a832a2590527100afc05fb9de2e332a263d52c19
Diff: https://reviews.apache.org/r/62179/diff/1/
Testing (updated)
-------
precheckin running
Thanks,
Jinmei Liao